Say I have a web service with a method MyWebServiceMethod(string passedValue).
The web service calls a method MyServiceMethod(string passedValue) where the value from the web service is passed along.
The service calls a repository method MyRepositoryMethod(string passedValue) where again, the value from the service is passed along.
I hope that is clear enough. I can post code if need be.
My question is around null argument checking.
Should I check passedValue and throw an exception in:
- The Web Service
- The Service
- The Repository
Obviously, if passedValue is null in the web service, the following methods will never get invoked but should I check in case the solution changes in the future?
Edit
I should have said that I don’t expect null to ever be a valid value.
Also, My worry in these situations is that someone may change the web service or create another client with no argument checking that will call the service with invalid arguments. So I tend to think the checking should be in both places. Then I think of YAGNI and wonder if we should cross that bridge when we come to it.
What is considered best practice in these situations?
1
If null is not valid on the web service, then check it in the web service.
If it ever becomes valid, update the code. Don’t worry about implementing things that do not currently exist and you don’t know if they ever will exist.
You already know that null is invalid, so catch it immediately.
If there’s only one parameter and null becomes valid you may just want to create a parameterless method as an option in the future so that your user won’t need to pass null to get the functionality they need.
Personally, it depends on what is going on in the methods. If the middle method is just passing it down, then I wouldn’t check there. The method that needs to validate the parameter does the check. Even if the parameter is checked upstream, the method that needs a valid parameter should check.
If passedValue is used in either MyWebServiceMethod or MyServiceMethod in any way aside from calling MyServiceMethod and MyRepositoryMethod then you’ll need to check it in those places so that whatever code in each of those layers that consumes passedValue has a valid value to work with.
If, on the other hand, at each layer passedValue is only used to call into the next layer, check it on the innermost layer and allow the exceptions to bubble up to the user from there. That way you only have one place to maintain the code, should the expectations change.
Aside from that, don’t worry too much about future consumers of any layer. If MyServiceMethod changes later to use passedValue in other work, the developer who adds that functionality should understand the implications of that change and protect themselves accordingly.
In my opinion, it depends greatly on how the code is going to be used. If the code (classes, etc.) you are creating will be reused in multiple different ways — or by other people — then you might want to have multiple levels of checking (as appropriate — it may be OK to just let a null reference exception happen). But if it’s more likely to be kept to a small team (or yourself) then you can handle it wherever you want, and earlier may be better than later.
In my mind, it’s the same kind of idea as whether you should do null checks. Some people say to do them anywhere something could possibly be null because this is “defensive programming.” But if you control the code and know that null won’t happen, then the checks are just a waste.
Should I check passedValue and throw an exception in:
- The Web Service
- The Service
- The Repository
What are you going to do if the string is null? Throw an illegal argument exception to prevent a null pointer exception?
I would not check unless failure to do so could lead to a security hole. A null pointer exception followed by a 404 or other error is a reasonable response to a user who types in an invalid URL for a web service call. It is also a reasonable response when a web page constructs an incorrect URL. A null pointer exception is a reasonable response to a developer who passes a null argument to a function that is not documented to accept a null.
There are many ways for an argument to be unusable. An object reference can be null. A floating point value can be infinite, or NaN. An output stream may not be open. Exceptions were invented to eliminate the need to check all these conditions at every level of the code.
2 solutions :
– null is or may become a valid value for your service. In that case you have to pass it along your stack.
– If null value is forbidden, the basic validation is better at the upper level (webservice) to avoid unnecessary calls.
Given the case, that you have a method that only calls another method and passes the arguments, at that point it is not necessary to check if a variable is null.
When the repository is called, you have to decide if null is a valid value or not. If not not, you have to test your variable against null and throw an exception.
As long as a “layer” expose a public interface with a contract, the null value should be checked. So in your case you should have 3 null checks.
This will allow you to be fool proof if MyServiceMethod or MyRepositoryMethod are invoked from somewhere else.