This week, I’ve lived again an experience from a few years ago, but in the opposite seat.
As a software architect/team leader/technical lead (select the term you’re more comfortable with), I was doing code reviews on an project we were working on and I stumbled upon a code that looked like that:
public void someMethod(Type parameter) {
if (parameter == null) {
throw new NullPointerException("Parameter Type cannot be null");
}
// Rest of the method
}I was horribly shocked!
An applicative code throwing a NullPointerException, that was a big coding mistake.
So I gently pointed to the developer that it was a bad idea and that I’d like him to throw an IllegalArgumentException instead, which exactly the exception type matching the use-case.
This was very clear in my head, until the dev pointed me the NullPointerException Javadoc.
For simplicity’s sake, here’s the juicy part:
Thrown when an application attempts to use
nullin a case where an object is required. These include:
- Calling the instance method of a
nullobject.- Accessing or modifying the field of a
nullobject.- Taking the length of
nullas if it were an array.- Accessing or modifying the slots of
nullas if it were an array.- Throwing
nullas if it were aThrowablevalue.Applications should throw instances of this class to indicate other illegal uses of the
nullobject.
Read the last sentence again:
"Applications should throw instances of this class to indicate other illegal uses of the null object".
It seems to be legal for an application to throw an NPE, even recommended by the Javadocs.
In my previous case, I read it again and again… And ruled that yeah, it was OK for an application to throw an NPE. Back to today: a dev reviewed the pull request from another one, found out it threw an NPE and ruled it was bad practice. I took the side of the former dev and said it was OK.
What would be your ruling and more importantly, why?
Note: whatever the decision, this is not a big deal and probably not worth the time you spend bickering about it 😉