Code Advice #13: Don't spot-fix null pointer exceptions

WARNING: This blog entry was imported from my old blog on blogs.sun.com (which used different blogging software), so formatting and links may not be correct.

(See intro for a background and caveats on these coding advice blog entries.)

One pattern I've seen some developers use is to respond to NPE (NullPointerException) bugs by inserting a null pointer check at the location of the NPE. Frequently this will be accompanied by some sort of comment like "Why is this null?".

My advice is simple: Leave the NPE until you've investigated the root cause. You need to have a null-policy. Decide which references can be null, and which can't. The problem with spot fixing an NPE is that in many cases, you've only moved the bug from one place to another. And in particular, you've moved the symptom further from the cause.

First, decide whether for your own methods, you ever allow null as a return value. Instead of returning null for an empty or uninitialized list for example, always return an empty list. That doesn't mean you have to create an "unnecessary" object to indicate null; you can call Collections.emptyList(). This will return a shared object which is used for all empty lists - and through generics you still get to keep type checks:

public static final <T> List<T> emptyList() { ... }

You also need to make sure that you handle nulls properly at the boundaries of your own code. If you're calling a library, make sure that the return values are only null if your program universally allows null for the particular piece of data being passed around. If you're providing an API, make sure that you either handle null parameters (if you want to be lenient), or throw IllegalArgumentExceptions if you expect clients to be well behaved. (I don't throw NullPointerException.)

This is clearly asking a lot of you. You need to keep track of which parameters and fields are allowed to be null. For that reason, it's smart to pick a simple policy. However, with tools we should be able to do better. Annotations are ideal for this. You can annotate parameters and fields to indicate whether or not they are allowed to be null. With that information, tools can check whether your code is correct (and even without tools, when you're calling an API, these annotations help document what is expected of you in a more accurate way than English javadoc descriptions, which frequently say nothing on the subject.) For example, if the static checker discovers that you are calling a method with a null parameter where that parameter is known to not be nullable, it can complain and alert you to the potential problem.

Annotations for nullability will probably find their way into the standard set of annotations. For now, you'll need to use some third party libraries and sprinkle your code with third party class names. That may be a bitter pill to swallow.
(Update two days later: See this entry for a solution.)
On the other hand, it may pay off. These annotations allow you to state your intent, and without that additional information from the programmer, tools cannot easily help you track down NPE problems.

Here's findbugs' set of annotations related to null checking - CheckForNull, Nullable, NonNull. Obviously, findbugs can also do the static analysis to alert you to errors in your program where you are violating the constraints. IntelliJ has support for this built in. It would be nice if NetBeans would do the same, but on the other hand, NetBeans tries hard to follow the JSR standards closely, so perhaps this will have to wait until we have java.lang.Nullable. In the mean time you can use the findbugs NetBeans plugin, available from nbextras.org. Just grab the annnotations.jar and add it to your project, and then code completion will find the annotations and handle imports for you.