CODEWITHSUNDEEP
javaobjectnullpointerexceptionnull
Harshit
Harshit

Reputation: 1263

Java : When to skip null checks on an object?

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

  1. Doesn't it makes sense to allow our code to fail through an NPE if we cannot recover from that null state? (In the example above, it's an irrecoverable scenario it seems.)
  2. If not, why?

Upvotes: 10

Views: 1368

Answers (4)

Tom
Tom

Reputation: 3336

I wrote a personal class ArgumentChecker.java, containing many frequently used simple static methods for exception checking, like checkPositive(double value), checkEquals(int v1, int v2), checkNonDecreasingOrder(int... args), checkNonNull(Object obj), ..., etc. Two examples are like below. 

public static void checkAllEqualsTo(int theValue, int... values){
    for (int i = 0; i < values.length; i++) { // StringUtils is also a personal simple class containing some wrapped methods
        if (values[i] != theValue)
            throw new IllegalStateException(NOT_ALL_EQUAL_EXCEPTION + "; values = " + StringUtils.toString(values, values.length) + ", theValue = " + theValue);

    }
}

public static void checkAllEquals(int... values){
    if (values.length == 0){
        return;
    }
    checkAllEqualsTo(values[0], values);
}

These methods are reusable, and method calls to these exception checking methods are also concise and readable.

For this question, the code becomes: 

ArgumentCheck.checkNonNull(object);
object.someMethod();

Upvotes: 0

yshavit
yshavit

Reputation: 43391

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:

  1. When I want to throw a checked exception instead of unchecked.
  2. When I want to check for the null before I actually use the reference.

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.

Upvotes: 8

matthieu.cham
matthieu.cham

Reputation: 511

NPE are considered bad because they give no "semantic" to the problem that has just occurred. About any piece of code in a Java app may throw a NPE. So when it occurs, you have no immediate clue about what causes the NPE : a missing user entry ? a programming error ? A misused external dependency ?

Because of this variety of possible causes, it would be very bad practice to catch NPE at the upper level to handle the issue

So, when you have the possibility, like in the example you gave, at the time you're writing the code, you are the one who knows what does it means when object == null. So if you choose to throw an exception with a semantic meaning, you can catch it specifically at upper level, and treat this special case functionally.

Regarding the boilerplate if (... == null) else ... , you can avoid it if you use Java 8 and Optionals

Upvotes: 3

Abhijit Sarkar
Abhijit Sarkar

Reputation: 24528

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.

Upvotes: 4

Related Questions