How can unit testing make parameter validation redundant?

Question

We have a convention to validate all parameters of constructors and public functions/methods. For mandatory parameters of reference type, we mainly check for non-null and that's the chief validation in constructors, where we set up mandatory dependencies of the type.

The number one reason why we do this is to catch that error early and not get a null reference exception a few hours down the line without knowing where or when the faulty parameter was introduced. As we start transitioning to more and more TDD, some team members feel the validation is redundant.

Uncle Bob, who is a vocal advocate of TDD, strongly advices against doing parameter validation. His main argument seems to be "I have a suite of unit tests that makes sure everything works".

But I can for the life of it just not see in what way unit tests can prevent our developers from calling these methods with bad parameters in production code.

Please, unit testers out there, if you could explain this to me in a rational way with concrete examples, I'd be more than happy to seize this parameter validation!

Answer 1

My answer is "it can't." Basically it sounds like I disagree with Uncle Bob on this (amongst other things).

It's all too easy to imagine a situation where you've unit tested your library code for non-null arguments, and you've unit tested your calling code for a path which happens to provide a null argument to the library without you being aware of it, but which also happens not to cause any problems for that particular path. You can have 100% coverage and actually a pretty good set of tests, and still not notice the problem.

Is everything fine? No, of course it isn't - because you're violating the library contract (don't give me a non-null value) without being aware of it. Can you be comfortable that the only situations in which you're providing a null argument are ones where it won't matter? I don't think so - especially if you weren't even aware that the argument was null.

In my view, public APIs should validate their arguments regardless of whether the calling code and the API itself is unit tested. Problems in calling code should be exposed, and exposed as early as possible.

Answer 2

That's a question I've been asking myself for ages, and still haven't got a satisfying answer to.

But I believe that when it comes to argument validation, you need to distinguish between two cases:

1. Are you validating the argument to catch logical programming errors?

   if (foo == null) throw new ArgumentNullException("foo");
   

   is quite likely an example of that.

2. Are you validating the argument because it is some external input (supplied by the user, or read from a configuration file, or from a database), which could be invalid and must be rejected?

   if (customerDateOfBirth == new DateTime(1900, 1, 1)) throw …;
   

   might be of this type of argument check.

(If you're exposing an API consumed by someone outside your team, point 2 roughly applies as well.)

I suspect that methodologies such as unit testing, design by contract, and to some extent "fail early" focus mostly on the first type of argument validation. That is, they attempt to detect logical programming errors, not invalid input.

If that is the case, then I dare say it doesn't actually matter which method of error detection you follow; each has its own advantages and disadvantages.^† In the extreme case (for instance, when you have absolute trust in your abilities to write bug-free code), you could even drop these checks completely.

However, whatever method you choose for detecting logical errors in your code, you still need to validate user input etc., thus the need to distinguish between the two kinds of argument checks.

---

^†) An amateur's incomplete attempt at comparing the relative advantages and disadvantages of Design by Contract, unit testing, and "fail early":

(Though you didn't ask for it... I'll just mention a few key differences.)

Fail early (e.g. explicit argument validation at start of method):

• writing basic checks such as guards against null are easy to write
• might mix up guards against logical errors and validation of external input with the same syntax
• doesn't allow you to test the interaction of methods
• does not encourage you to define (and thus think about) your methods' contracts rigorously

Unit testing:

• allows you to test code in isolation, without running the actual application, so detecting bugs can be quicker
• if a logical error occurs, you won't have to trace the stack to find the cause, because each unit test stands for a specific "use case" of your code.
• allows you test more than just single methods, e.g. even the interaction between several objects (think stubs & mocks)
• writing easy tests (such as guards against null) is more work than with the "fail early" approach (if you strictly adhere to the Arrange-Act-Assert pattern)

Design by Contract:

• forces you to explicitly state the contract of your classes (though this is possible with unit tests, too — just in a different way)
• allows you to easily state class invariants (internal conditions that must always hold true)
• not as well supported by many programming languages / frameworks as the other approaches

Answer 3

It all depends on the type of application you are developing.

1. I have spent most of my time writing applications that do not expose public APIs, in this case, the application must be deterministic in a sense that all parameters must and will be different than null. In a nutshell, you should be performing input validation at your system boundaries not to let these invalid inputs sneak into your application which might end up in null references and such. In this kind of application, you have full control of checking your application's input right where you acquire them.

2. If you are writing public APIs, then not checking for null references is not recommended. Just have a look at all the MSDN class methods that can throw exceptions, all of that happens inside the API as precondition checks, you can read the C# Framework design guidelines for more info.

In my opinion, be it an exposed (or not) API application, having preconditions for your methods is always a good thing (those contracts are documentations for your peers who will work on your code in the future)

Answer 4

I aggree with Uncle Bob on almost everything, but this not this one. I vote for the "fail fast and fail hard"-policy.