modularizing null pointer checking

[Udayan Kumar]

Hi there,

In java most of the time we intend to do a lot of defensive programming having null checks each and every time we attempt de-reference an object.
Can this can be modularized i.e. instead of having the null pointer checks scattered across we can just invoke this utility method which kind of returns null indicator flag.

Do post pointers or your thoughts on the same.

Thanks,

Uday

[Rob Spoor]

You mean replace the following with one single line?

if (myobject != null)
{
    myobject.doSomething(parameter);
}

You could use reflection but that's just evil. Or you could create a helper method for each call, but that can lead to dozens or even hundreds of helper methods.

The only helper method I have is for checking if an object is null, and throwing a NullPointerException if it is:

public static <T> T checkNull(T object, String message)
{
    if (object == null)
    {
        throw new NullPointerException(message);
    }
    return object;
}
 
// use:
File file = checkNull(myFileParameter, "file cannot be null");

Saves me 3 lines of code each time

[Campbell Ritchie]

I disagree.
You don't want null checks all the time. There are certain situations where nulls are legitimate values, eg

In the "finally" when closing a Reader, Writer, SQL Connection, etc.

In a binary tree; adding a value always adds two nulls.

In a method which accepts nulls, eg the 1st argument to most JOptionPane#showXXXDialog methods.

In a method which might return nulls, eg Map#get, Queue#poll

There are bound to be others. But we know what they are; the methods usually mention null in their Javadoc, and we know a Reader might be null because we write BufferedReader inBuff = null; before writing "try." Other methods say null is prohibited; in the String class API documentation it says "Unless otherwise noted, passing a null argument to a constructor or method in this class will cause a NullPointerException to be thrown." But I would have preferred to see "null not permitted" for each method.

If you start getting nulls anywhere else, you have done things like forgetting to initialise objects in your constructor, or let somebody pass null incorrectly, or passed null yourself. All these are errors in your code, which is what the Java Tutorial says.

Therefore: lots of if (obj != null) obj.foo(); type statements are simply a way of hiding the fact that you have mistakenly allowed null values to get into your code. Better to get a NullPointerException and sort out the error than to allow other errors to accumulate unnoticed. Notice what Rob's method does: it throws an Exception. Exactly what you want.

Therefore: you don't want a nullTest method.

[Jeanne Boyarsky]

Not only am I with Campbell on this, but I blogged on Friday about why checking for nulls everywhere is not a good thing.

[Jeanne Boyarsky]

Rob,
What does that buy you? Java will throw a null pointer if the object is null without you doing anything extra.

I do actually have a similar utility method to yours. But it throws a different type of exception - not the one Java gives automatically. I use this utility method to validate input parameters coming from callers outside my control.

public static void validateNotNull(Object object, String message)
{
    if (object == null)
    {
        throw new IllegalArgumentException(message);
    }
}

[Ilja Preuss]

Originally posted by Jeanne Boyarsky:
Rob,
What does that buy you? Java will throw a null pointer if the object is null without you doing anything extra.

Well, one reason *I* use a similar thing is to fail earlier. For example, when parameters that get passed into a constructor will only get used in much later method calls.

[Ilja Preuss]

And for the cases where you use null to actually signal a special case, think about using the Null Object design pattern instead.

[Rob Spoor]

Originally posted by Ilja Preuss:


Well, one reason *I* use a similar thing is to fail earlier. For example, when parameters that get passed into a constructor will only get used in much later method calls.

Exactly. I don't use that method all the time, just in the cases Ilja described - as a check in a constructor or setter to make sure that the object will not cause problems at a later time.

I am also perfectly aware that sometimes you can't avoid having to check null. I also have written some other utility methods just for closing Closeable, Connection, Statement, ResultSet, etc with included null check.

[Ilja Preuss]

Originally posted by Rob Prime:

I also have written some other utility methods just for closing Closeable, Connection, Statement, ResultSet, etc with included null check.

Ditto. Isn't it annoying that there isn't a Closable interface in Java...?

[Rob Spoor]

There is in java.io, but it would be nice if there were one in java.sql as well.

The difference in the two would be the exception they can throw.

[Mike Simmons]

[Jeanne]: What does that buy you? Java will throw a null pointer if the object is null without you doing anything extra.

In addition to failing earlier, a method like checkNull() is clearer, in two ways. A NullPoniterException thrown by Java doesn't always tell you which variable is null - there may be more than one possible cause on a given line, so the line number isn't always enough info. With checkNull() the stack trace will be enough info. You can even pass in the name of the variable so that it will be part of the error message.

The second way it's clearer is in communicating intent. With an automatically-thrown exception, a programmer unfamiliar with the code may follow the stack trace to a line like this:

foo.bar();

and decide that the best way to fix this is with something like

if (foo != null) {
  foo.bar();
}

But in many cases the person who wrote the method may know that the method really needs a non-null value, and they can communicate that clearly by inserting a check that intentionally throws an exception. I've used a variant like this:

public static void requireNotNull(Object obj, String name) {
  if (obj == null)
    throw new NullPointerException("Variable " + name + " may not be null.");
}

This should at least cause other programmers to think twice before simply inserting a "if (foo != null)" into a method.

(And I agree with everyone who said that they often don't want null checks everywhere, as they often hide errors and clutter up code.)

I'm hoping that JSR 308 comes along and enables us to specify this sort of thing with annotations - though annotations themselves need a little more power to be fully useful here.

[Ilja Preuss]

Originally posted by Mike Simmons:
In addition to failing earlier, a method like checkNull() is clearer, in two ways. A NullPoniterException thrown by Java doesn't always tell you which variable is null - there may be more than one possible cause on a given line, so the line number isn't always enough info. With checkNull() the stack trace will be enough info. You can even pass in the name of the variable so that it will be part of the error message.

Just splitting up the single line into several lines so that the stacktrace is more specific sounds simpler to me...

The second way it's clearer is in communicating intent. With an automatically-thrown exception, a programmer unfamiliar with the code may follow the stack trace to a line like this:

foo.bar();

and decide that the best way to fix this is with something like

if (foo != null) {
  foo.bar();
}

Programmers who do that need to be shot, as an example to others...

[Campbell Ritchie]

Originally posted by Ilja Preuss:
Programmers who do that need to be shot, as an example to others...

They most certainly shouldn't be shot. It's far too quick and painless.

[Mike Simmons]

[Ilja]: Just splitting up the single line into several lines so that the stacktrace is more specific sounds simpler to me...

That's an option, but I increasingly prefer the brevity that method chaining prefers. Of course, brevity can also be a reason to omit non-necessary calls like requireNotNull() in the first place. Which is why I like the idea of using annotations. I want to be able to specify that the default in a given file is for all variables to be @NotNull, then put a @Null or @Nullable annotation on the few exceptions. Then let FindBugs do its thing. Or maybe use AOP to insert all the requireNotNull() calls at the beginning of each method.

[Ilja]: Programmers who do that need to be shot, as an example to others...

[Campbell]: They most certainly shouldn't be shot. It's far too quick and painless.


[ October 23, 2008: Message edited by: Mike Simmons ]

[Jeanne Boyarsky]

I agree with the principle of failing earlier. I think the only difference is I would throw IllegalArgument rather than NullPointer. I'm suspecting now that NullPointer is more correct. Anyone know why?

[Mike Simmons]

I think either one could be acceptable, based on the documentation for the two exceptions - Sun didn't really make this as clear as they might have. But in practice, Sun's own libraries heavily prefer to throw NPE rather than IAE in this situation. And as a result, NPE is better because that's what most Java programmers have been trained to expect at this point.

There was a lengthy discussion of this back here that may interest you.

[Ilja Preuss]

Originally posted by Jeanne Boyarsky:
I think the only difference is I would throw IllegalArgument rather than NullPointer. I'm suspecting now that NullPointer is more correct. Anyone know why?

Personally, I don't care either way.