I have been using a lot of defensive null checks within my Java code. Though they serve their purpose well (most of the time), they come a with a huge trade off with 'ugly' looking code.
Does it really make sense to put these null checks in all the time? For example:
if(object == null){
log.error("...")
throw new SomeRuntimeException("");
} else{
object.someMethod();
}
Practically speaking, the above piece of code is equivalent to the statement object.someMethod();
If object
's value is null, an exception would have been thrown in both cases (NullpointerException in the later).
Does it really make sense to mask the NullpointerExcetion (NPE) and throw some custom RunTimeException (as in the above snippet)? I have observed some developers treating NPE as evil, often trying to find out defensive ways to mask it and cover it up with some custom exception. Is this masking really required ?
Question
The best way is to only check when necessary. If your method is private , for example, so you know nobody else is using it, and you know you aren't passing in any nulls, then no point to check again. If your method is public though, who knows what users of your API will try and do, so better check. If in doubt, check.
Avoiding Null Checks Through Coding PracticesIt's usually a good practice to write code that fails early. So, if an API accepts multiple parameters that aren't allowed to be null, it's better to check for every non-null parameter as a precondition of the API.
In a nutshell, the Optional class includes methods to explicitly deal with the cases where a value is present or absent. However, the advantage compared to null references is that the Optional class forces you to think about the case when the value is not present.
One way of avoiding returning null is using the Null Object pattern. Basically you return a special case object that implements the expected interface. Instead of returning null you can implement some kind of default behavior for the object. Returning a null object can be considered as returning a neutral value.
In cases like you posted, there's no benefit to the check. You're replacing one RuntimeException with another one, which has no extra information or value (and arguably a bit less to someone who's not familiar with your code, since everyone knows what an NPE is, and not everyone knows what your SomeRuntimeException is).
The main two times I'd consider an explicit check are:
The second case is especially important when I'm only going to be storing the reference for later: in a constructor, for instance. In that case, by the time somebody uses the reference and triggers the NPE, it may be difficult to find how that null got there in the first place. By checking for it right before you save it in a field, you stand a better chance of finding the root cause. In fact, this is a common enough pattern that the JDK even has a requireNonNull
helper for it.
If you look at a lot of well-established, high-reputation projects, I think you'll find that the "just use it, and let an NPE happen if it happens" pattern is common. To take one example off the top of my head, the code for the JDK's Collections.sort
is simply list.sort(null)
, where list
is the method argument. If it's null, that line will throw an NPE.
For internal code, a bunch of null checks is pretty useless. If you're passing null to a method that doesn't expect it, just letting it fail with NPE is acceptable than trying to guard against a mistake. If your code is called from other code that you don't control, you can either assert in the beginning of the method (Objects.requireNonNull), or provide a Javadoc that states the consequence of passing in null. The later approach is prevalently used in the JDK codebase.
If you love us? You can donate to us via Paypal or buy me a coffee so we can maintain and grow! Thank you!
Donate Us With