The architecture for the webapp we’re building at work is layered in the typical way for Java applications: Our main layers are presentation, RPC, business and persistence. So when a user clicks some button on our app in his browser his request will most commonly pass all these named layers; the presentation layer (which actually runs on his browser since we’re using Google Web Toolkit) passes some value to the RPC layer which does authorization checks for the action the user would like to perform, transforms the value and passes it to the business layer which eventually passes it to persistence causing sth. to be stored in the database. When one of our programmers implements a feature (e.g. “user edits his profile”) he builds the feature from presentation down to persistence. This has proven as a very robust and efficient development model but it poses a problem which every of our programmers must be aware of:
Every delivery of a value from one layer to the next implies the risk of passing it the “wrong way”(TM). The most common problem I’ve dug up in the last few months by looking at the logs in our dev, testing and production systems was that at many points there are passed null values from one layer to the other causing a NullPointerException (NPE for short) to appear in the logs. This comes from the fact that in some rare cases you develop a feature and know exactly how you call all the methods in the different layers because you’re building them right from the ground just for that feature (at least, for now); At times you don’t take into account the fact that someone else at a later point in time will use your method in a totally different way. Most of the time our code doesn’t check parameters for null even when the API implicitly forbids passing null values. And this is in my humble opinion a good thing when the callers know exactly that they have to be careful what they feed the next layer with.
Many young Java developers do try to solve the problem of NPEs in a very defensive programming style cluttering their code with null checks, assertions and Exception throwing. Some examples:
This behaviour is absolutely understandable in a way that you might want to prevent users from calling methods in a way that probably leads to odd behaviour of your particular code.
But the truth is: When one of your methods behaves odd because someone called it in a way the contract forbids then it is absolutely allowed to behave odd!
Every layer in a multi-tier application should be handled as if it was a closed, standalone API. It should be documented well, testable in isolation und define a clear contract via its signature (and/or documentation). Since I like to back my ideas with proofs of concept I looked at the implementations of very well designed APIs like the Java Class Library. Take for example java.util.Collections. You will not find a single if(xyz == null) construct there when the API doesn’t explicitely allows passing null values and the comment at the top of the class file states: “The methods of this class all throw a NullPointerException if the collections or class objects provided to them are null.” I would even say this comment isn’t necessary since this behaviour should be the default for every API.
This design principle has a very important implication: Every caller of an API or - in my case - a service layer method must check if the parameters he hands over to the methods adhere to the API’s contract. Doing this inside of the API method would probably save some code repetition but at the time you implement the API you will never be able to program defensively around every possible corner case anyhow.
So what I want to recommend you very warmly today is this: Design your service layers as if you would build an isolated API and don’t even try to handle null arguments when it’s not absolutely necessary. This will lead to much better code since the programmers will start to think very carefully what they hand over as parameter.