This came up at work this week, so I thought I'd disgorge my views on the issue. The question at hand is whether or not every method needs to validate its input parameters.
For example, if I have a method
public class Example
{
public int DoThingy(SomeObject obj)
{
return obj.GetImportantValue();
}
}
This method depends on the value of obj not being null. If a caller passes a null, the framework will, on my behalf, throw a NullReferenceException. Is that OK? Sometimes it might be, and we'll come back to that.
The alternative is for me to validate my input parameters thusly:
public int DoThingy(SomeObject obj)
{
if (obj == null)
throw new ArgumentNullException("obj");
return obj.GetImportantValue();
}
The discussion we got into was wether this was really necessary or not, given that the Example object only got used in one place, and it would require extra code to validate the parameters. The assertion was made by more than one person that the NullReferenceException was just fine, and that there was no need to do any validation.
My $.02 on the matter was (and is) that if you don't do parameter validation, you are not upholding and/or communicating your contract. Part of the contract implied by DoThingy() is that you have to pass a non-null SomeObject reference. Therefore, in order to properly communicate your contract, you should throw an ArgumentNullException, which informs the caller exactly how they have violated there half of the contract. Yes, it's extra code. No, it may never be necessary. A whole different subject is whether of not "fewest possible lines of code" is a design goal. I'm going to avoid going there just now.
That said, there are obviously mitigating circumstances that apply. If the object in question is really only called in one place, upholding the contract is less of a concern, since you should be able to write tests that cover the right cases to make sure the caller never passes null. Although that brings up a separate issue. If the method only had one caller, which is it in a separate object at all? Again, we'll table that one. In addition, since in this particular case the DoThingy() method only takes one parameter, we don't have to wonder to hard when we get a NullReferenceException where the culprit is.
The other issue besides contract is debugging. If you don't check your input, and just let the method fail, then the onus is on the caller to figure out what the problem is. Should they have to work it out? If the method took 10 parameters, all reference types, and I let the runtime throw NullReferenceException, how long will it take the consumer to find the problem? On the other hand, if I validate the parameters and throw ArgumentNullException, the caller is informed which parameter is null, since the onus was on me to find the problem.
One last reason... If I validate my input parameters, it should leave me with fewer failure cases to test, since the error path is potentially much shorter than if the runtime handles it. Maybe not much of a savings, but it's there.
Thoughts?