Parameter Validation Responsibility: Caller or Callee?
I just picked up "the pragmatic programmer". It's a pretty good book, i'd recommend it.
Anyway, there's this page in the book (p115) that talks about "who is responsible for checking the precondition, the caller or the routine being called?" The authors say that it is the caller. For example, "Thesqrt function has a precondition--its argument must not be negative. If the user enters a negative number at the console, it is up to the calling code to ensure that it never gets passed tosqrt. ... [the calling code] could terminate, it could issue a warning and read another number, or it could make the number positive and append an 'i' to the result returned by sqrt. Whatever its choice, this is definately notsqrt's problem "
My first reaction was to disagree with this. I base my argument on a previous chapter that talked about the Don't Repeat Yourself axiom: If you let the caller be the one that checks the precondition, you'll likely need to repeat this check each time you call the method. You'll never want a negative number passed into thissqrt method, therefore, why would you want to have to check every single time you use it when the method itself could it once.
Faced with this issue, i've always consciously chosen the option I just spoke of. What do you guys think andwhy? I'm not really satisfied with the rational the authors gave in the book. Maybe some more examples would help me get it.
Thanks
[1510 byte] By [
tieTYTa] at [2007-10-2 1:34:22]
A method can never know all the possible ways in which it will be called. Therefore it needs to specify under which conditions it can be called and leave it at that. This is the contract of that method. It is then up to the caller to make sure it doesn't violate the preconditions of the contract when calling the method.
You say the sqrt() method can handle it, but what can it do? The only reasonable course of action under that contract would be (in Java terms) to throw an IllegalArgumentException, which the calling code would then have to handle anyway. This is similar to a precondition violation in languages like Eiffel: yes, there is a check on the input, but nothing useful happens unless you make it so.
It's not a matter of repeating yourself either: you want/need to do things differently when dealing with user input as opposed to dealing with input from, for example, a measuring system. Yes, you will need to check for a negative number in both cases, but that can't be helped. This doesn't violate the DRY principle either. If it did, you could only ever call any one method once ;) Of course, if the checks you need to do are very involved, you will probably want to move them into their own utility method so you can use them in multiple locations (this is what DRY is all about).
BTW: in Java, the sqrt() method will actually handle negative input (it will return NaN).
Just to add some more info:
In the Programming-by-Contract methodology, precondition violations are programming errors which will cause the program to stop. This forces you to check your arguments against the contract of a method before calling the method.
In Java, precondition violations are often implemented using exceptions, which can/may/must be handled by the caller after calling the method. This leads to less efficient code: you first call a method with an argument that might violate a precondition and if it does, you have to handle the exception, where you will probably need to check the value of the argument anyway (if you still can, because the exception might not tell you what the original value was).
I am not familiar with this book but im not sure if I like it. Don't repeat yourself seems a misleading oversimplification of some serious OO principles. And both the caller and the calle have a responsibility here. The caller needs to know what to expect from the callee. The callee needs to be able to defend itself from nonsense.
@Herko:> The only reasonable course of action under that contract would be (in Java terms) to throw an IllegalArgumentExceptionSo, is not the callee validating here? :^)- Saish
Saisha at 2007-7-15 18:57:30 >
in fact both violate.
The method being called throws an exception when it gets an illegal argument, the method calling handles that exception (or does its own validation before calling to ensure an illegal argument is never passed).
Either way will work, in my experience I often use a combination (like trying to parse an Integer from input in my validation code and showing an error message when I get a NumberFormatException, then passing the argument on to the data handling code in the glossy knowledge it passed validation and therefore won't throw a NumberFormatException when parsed in the business logic.
Depends on what you call validating. Somewhere, somehow, an exception or error will occur. Like _dnoyeB said: it's probably a good idea for the callee to catch these errors as soon as possible instead of waiting for it to go wrong somewhere deep down. In any case, it is still the caller's job to handle the situation. The only thing the callee can do is report the error (either by checking the input immediately or waiting until things go wrong deeper down).
@_dnoyeB: I do have the book and like I've tried to indicate in my first post, I don't think this is what the author meant by Don't Repeat Yourself. The DRY principle is sound, as long as you don't take it to mean you can only ever write any one line once. DRY means that as soon as you notice the same (or even a similar) sequence of lines in two or more separate pieces of code, your initial reaction should be to try and move them into a separate method.
There are also different levels of validation that are performed. Normally the Caller performs domain specific validations (ie the checks it performs are more strict). The callee ensures that it will not become corrupted using the arguments passed to it. This becomes more obvious when the example is an object with manipulation methods on it. The manipulative methods should ensure that arguments passed to them do not cause the object to become "invalid" while the caller of those methods will ensure that the parameters it passes will meet its business requirements.
example:
method Wibble.setIdentity(String id) accepts any string that is not null
the caller of setIdentity ensures that the "id" parmater it provides is not only not-null but is also not empty and is unique across the set of Wibbles. The caller uses its knowlege of the usage domain to restrict the values that are valid. Wibble does not care about the domain it is used in as long as it is given an ID
matfud
Actually, in Programming-by-Contract, the callee's responsibility for maintaining correctness only applies when the caller honors the contract. If the contract specifies that only non-null input is accepted, the callee is NOT responsible for what happens when a caller provides null
> Actually, in Programming-by-Contract, the callee's
> responsibility for maintaining correctness only
> applies when the caller honors the contract. If the
> contract specifies that only non-null input is
> accepted, the callee is NOT responsible for what
> happens when a caller provides null input.
And in "programming by belt and braces" you check that the caller is not being stupid. Especially if you are writing an API where you have little control over who uses it.
It is far more difficult to trace a problem caused by an object becoming corrupted then it is to notice the data being passed to said object was garbage.
Programming-by-Contract has some nice ideas. Most of the information about how to use a method/class is indeed in the documentation. Method signitures alone are not enough to code by. However that does not mean you should totaly ignore simple tests that can save you lots of time later.
If you can, in a simple way, enforce the contract then why not?
cheers
matfud
That's the point: the callee cannot enforce the contract. Enforcing the contract would mean adapting the input to fit what it can handle. The callee doesn't have enough knowledge to do this in the general case. The callee can only try to make sure it (and by extension: the caller) fails quickly and cleanly instead of at an undefined point later on. This is what preconditions do in Eiffel and what you yourself can do by throwing exceptions in Java.
The difference is that in Eiffel precondition violations are treated as stop-the-world errors (more like assertions in Java). Exceptions in Java can be caught and acted upon, which places additional (and often undocumented) responsibilities on the caller.
A callee must ensure that all arguments passed to it are valid. It must also ensure that its state is not corrupted during execution with valid arguments. That's it. The caller, by definition, must honor the contract the callee defines. Otherwise, an error (deterministic or non-deterministic) should be expected. What else is there?
- Saish
Saisha at 2007-7-15 18:57:30 >
If you follow the concepts of "caller is responsible" (which it impossible not to as if you don't your code will not work) then it often results in the developers of the callee absolving themselves of any responsibility for error checking. This is bad practice. Just saying the caller must not do this or that because of the contract does not mean the callee should not protest, strongly (with an exception), when the caller does do this or that.
Methods that fail silently have been the bane of my life recently (thank you very much apache). Especially when the "contract" is along the lines of
//@todo later insert javadoc
matfud
Oh yes, Another bit of a rant. Asserts (or stop-the-world) is not acceptable in some environments. You just want to let a transaction fail not sutdown the entire server. matfud
Then catch AssertionError in your front controller or Servlet. :^)- Saish
Saisha at 2007-7-15 18:57:30 >
I'd have both check.
with the coming of Web services, the server can't be sure that there IS a client that's doing validation. double bagging never hurts.
i think the client can check simple things (e.g., date format, required fields, etc.), but only the server should know what business valid input looks like. that's why you need checks for both. client checks make your apps less chatty; server checks make your apps safer.
%
