195

When reading JDK source code, I find it common that the author will check the parameters if they are null and then throw new NullPointerException() manually. Why do they do it? I think there's no need to do so since it will throw new NullPointerException() when it calls any method. (Here is some source code of HashMap, for instance :)

public V computeIfPresent(K key,
                          BiFunction<? super K, ? super V, ? extends V> remappingFunction) {
    if (remappingFunction == null)
        throw new NullPointerException();
    Node<K,V> e; V oldValue;
    int hash = hash(key);
    if ((e = getNode(hash, key)) != null &&
        (oldValue = e.value) != null) {
        V v = remappingFunction.apply(key, oldValue);
        if (v != null) {
            e.value = v;
            afterNodeAccess(e);
            return v;
        }
        else
            removeNode(hash, key, null, false, true);
    }
    return null;
}
13
  • 36
    a key point of coding is intent Commented May 12, 2017 at 2:57
  • 20
    This is a very good question for your first question! I have made some minor edits; I hope you don't mind. I also removed the thank-you and the note about it being your first question since normally that sort of thing isn't part of SO questions. Commented May 12, 2017 at 3:29
  • 12
    I'm C# the convention is to raise ArgumentNullException in cases like that (rather than NullReferenceException) - it's actually a really good question as to why you'd raise the NullPointerException explicitly here (rather than a different one). Commented May 12, 2017 at 3:44
  • 22
    @EJoshuaS It's an old debate whether to throw IllegalArgumentException or NullPointerException for a null argument. JDK convention is the latter.
    – shmosel
    Commented May 12, 2017 at 3:47
  • 34
    The REAL problem is that they throw an error and throw away all information that led to this error. Seems this IS the actual source code. Not even a simple bloody string message. Sad.
    – Martin Ba
    Commented May 12, 2017 at 8:59

10 Answers 10

272

There are a number of reasons that come to mind, several being closely related:

Fail-fast: If it's going to fail, best to fail sooner rather than later. This allows problems to be caught closer to their source, making them easier to identify and recover from. It also avoids wasting CPU cycles on code that's bound to fail.

Intent: Throwing the exception explicitly makes it clear to maintainers that the error is there purposely and the author was aware of the consequences.

Consistency: If the error were allowed to happen naturally, it might not occur in every scenario. If no mapping is found, for example, remappingFunction would never be used and the exception wouldn't be thrown. Validating input in advance allows for more deterministic behavior and clearer documentation.

Stability: Code evolves over time. Code that encounters an exception naturally might, after a bit of refactoring, cease to do so, or do so under different circumstances. Throwing it explicitly makes it less likely for behavior to change inadvertently.

5
  • 14
    Also: this way the location from where the exception is thrown is tied to exactly one variable that is checked. Without it, the exception might be due to one of multiple variables being null. Commented May 12, 2017 at 6:44
  • 46
    Another one: if you wait for NPE to happen naturally, some in-between code might already have changed your program's state through side-effects.
    – Thomas
    Commented May 12, 2017 at 10:37
  • 7
    While this snippet doesn't do it, you could use the new NullPointerException(message) constructor to clarify what is null. Good for people who have no access to your source code. They even made this a one-liner in JDK 8 with the Objects.requireNonNull(object, message) utility method.
    – Robin
    Commented May 12, 2017 at 13:05
  • 3
    The FAILURE should be near the FAULT. "Fail Fast" is more than a rule of thumb. When wouldn't you want this behavior? Any other behavior means you are hiding errors. There are "FAULTS" and "FAILURES". The FAILURE is when this programs digests a NULL pointer and crashes. But that line of code is not where FAULT lies. The NULL came from somewhere -- a method argument. Who passed that argument? From some line of code referencing a local variable. Where did that... See? That sucks. Whose responsibility should it have been to see a bad value getting stored? Your program should have crashed then. Commented May 12, 2017 at 16:19
  • 4
    @Thomas Good point. Shmosel: Thomas's point may be implied in the fail-fast point, but it's kind of buried. It's an important enough concept that it has its own name: failure atomicity. See Bloch, Effective Java, Item 46. It has stronger semantics than fail-fast. I'd suggest calling it out in a separate point. Excellent answer overall, BTW. +1 Commented May 13, 2017 at 18:31
40

It is for clarity, consistency, and to prevent extra, unnecessary work from being performed.

Consider what would happen if there wasn't a guard clause at the top of the method. It would always call hash(key) and getNode(hash, key) even when null had been passed in for the remappingFunction before the NPE was thrown.

Even worse, if the if condition is false then we take the else branch, which doesn't use the remappingFunction at all, which means the method doesn't always throw NPE when a null is passed; whether it does depends on the state of the map.

Both scenarios are bad. If null is not a valid value for remappingFunction the method should consistently throw an exception regardless of the internal state of the object at the time of the call, and it should do so without doing unnecessary work that is pointless given that it is just going to throw. Finally, it is a good principle of clean, clear code to have the guard right up front so that anyone reviewing the source code can readily see that it will do so.

Even if the exception were currently thrown by every branch of code, it is possible that a future revision of the code would change that. Performing the check at the beginning ensures it will definitely be performed.

24

In addition to the reasons listed by @shmosel's excellent answer ...

Performance: There may be / have been performance benefits (on some JVMs) to throwing the NPE explicitly rather than letting the JVM do it.

It depends on the strategy that the Java interpreter and JIT compiler take to detecting the dereferencing of null pointers. One strategy is to not test for null, but instead trap the SIGSEGV that happens when an instruction tries to access address 0. This is the fastest approach in the case where the reference is always valid, but it is expensive in the NPE case.

An explicit test for null in the code would avoid the SIGSEGV performance hit in a scenario where NPEs were frequent.

(I doubt that this would be a worthwhile micro-optimization in a modern JVM, but it could have been in the past.)


Compatibility: The likely reason that there is no message in the exception is for compatibility with NPEs that are thrown by the JVM itself. In a compliant Java implementation, an NPE thrown by the JVM has a null message. (Android Java is different.)

0
20

Apart from what other people have pointed out, it's worth noting the role of convention here. In C#, for example, you also have the same convention of explicitly raising an exception in cases like this, but it's specifically an ArgumentNullException, which is somewhat more specific. (The C# convention is that NullReferenceException always represents a bug of some kind - quite simply, it shouldn't ever happen in production code; granted, ArgumentNullException usually indicates a bug too, but it could be a bug more along the line of "you don't understand how to use the library correctly").

So, basically, in C# NullReferenceException means that your method actually tried to use it, whereas ArgumentNullException it means that it recognized that the value was wrong and it didn't even bother to try to use it. The implications can actually be different (depending on the circumstances) because ArgumentNullException means that the method in question didn't have side effects yet (since it failed the method preconditions).

Incidentally, if you're raising something like ArgumentNullException or IllegalArgumentException, that's part of the point of doing the check: you want a different exception than you'd "normally" get.

Either way, explicitly raising the exception reinforces the good practice of being explicit about your method's pre-conditions and expected arguments, which makes the code easier to read, use, and maintain. If you didn't explicitly check for null, I don't know if it's because you thought that no one would ever pass a null argument, you're counting it to throw the exception anyway, or you just forgot to check for that.

8
  • 4
    +1 for the middle paragraph. I would argue that the code in question should 'throw new IllegalArgumentException("remappingFunction cannot be null");' That way it's immediately apparent what was wrong. The NPE shown is a bit ambiguous. Commented May 12, 2017 at 17:20
  • 1
    @ChrisParker I used to be of the same opinion, but it turns out that NullPointerException is intended to signify a null argument passed to a method expecting a non-null argument, in addition to being a runtime response to an attempt to dereference null. From the javadoc: “Applications should throw instances of this class to indicate other illegal uses of the null object.” I’m not crazy about it, but that appears to be the intended design.
    – VGR
    Commented May 12, 2017 at 17:33
  • 1
    I agree, @ChrisParker - I think that that exception's more specific (because the code never even tried to do anything with the value, it immediately recognized that it shouldn't use it). I like the C# convention in this case. The C# convention is that NullReferenceException (equivalent to NullPointerException) means that your code actually tried to use it (which is always a bug - it should never happen in production code), vs. "I know the argument is wrong, so I didn't even try to use it." There's also ArgumentException (which means that the argument was wrong for some other reason). Commented May 12, 2017 at 21:15
  • 2
    I'll say this much, I ALWAYS throw an IllegalArgumentException as described. I always feel comfortable flouting convention when I feel like the convention is dumb. Commented May 12, 2017 at 21:19
  • 1
    @PieterGeerkens - yeah, because NullPointerException line 35 is SO much better than IllegalArgumentException("Function can't be null") line 35. Seriously? Commented May 15, 2017 at 20:44
12

It is so you will get the exception as soon as you perpetrate the error, rather than later on when you're using the map and won't understand why it happened.

9

It turns a seemingly erratic error condition into a clear contract violation: The function has some preconditions for working correctly, so it checks them beforehand, enforcing them to be met.

The effect is, that you won't have to debug computeIfPresent() when you get the exception out of it. Once you see that the exception comes from the precondition check, you know that you called the function with an illegal argument. If the check were not there, you would need to exclude the possibility that there is some bug within computeIfPresent() itself that leads to the exception being thrown.

Obviously, throwing the generic NullPointerException is a really bad choice, as it does not signal a contract violation in and of itself. IllegalArgumentException would be a better choice.


Sidenote:
I don't know whether Java allows this (I doubt it), but C/C++ programmers use an assert() in this case, which is significantly better for debugging: It tells the program to crash immediately and as hard as possible should the provided condition evaluate to false. So, if you ran

void MyClass_foo(MyClass* me, int (*someFunction)(int)) {
    assert(me);
    assert(someFunction);

    ...
}

under a debugger, and something passed NULL into either argument, the program would stop right at the line telling which argument was NULL, and you would be able to examine all local variables of the entire call stack at leisure.

2
  • 1
    assert something != null; but that requires the -assertions flag when running the app. If the -assertions flag isn't there, the assert keyword will not throw an AssertionException Commented May 14, 2017 at 8:47
  • I agree, that's why I prefer the C# convention here - null reference, invalid argument, and null argument all generally imply a bug of some kind, but they imply different kinds of bugs. "You're trying to use a null reference" is often very different than "you're misusing the library." Commented May 14, 2017 at 19:05
7

It's because it's possible for it not to happen naturally. Let's see piece of code like this:

bool isUserAMoron(User user) {
    Connection c = UnstableDatabase.getConnection();
    if (user.name == "Moron") { 
      // In this case we don't need to connect to DB
      return true;
    } else {
      return c.makeMoronishCheck(user.id);
    }
}

(of course there is numerous problems in this sample about code quality. Sorry to lazy to imagine perfect sample)

Situation when c will not be actually used and NullPointerException will not be thrown even if c == null is possible.

In more complicated situations it's becomes very non-easy to hunt down such cases. This is why general check like if (c == null) throw new NullPointerException() is better.

1
  • Arguably, a piece of code which works without a database connection when it's not really needed is a good thing, and the code which connects to a database just to see if it can fail to do so is usually quite annoying. Commented May 15, 2017 at 10:48
5

It is intentional to protect further damage, or to getting into inconsistent state.

0
2

Apart from all other excellent answers here, I'd also like to add a few cases.

You can add a message if you create your own exception

If you throw your own NullPointerException you can add a message (which you definitely should!)

The default message is a null from new NullPointerException() and all methods that use it, for instance Objects.requireNonNull. If you print that null it can even translate to an empty string...

A bit short and uninformative...

The stack trace will give a lot of information, but for the user to know what was null they have to dig up the code and look at the exact row.

Now imagine that NPE being wrapped and sent over the net, e.g. as a message in a web service error, perhaps between different departments or even organizations. Worst case scenario, no one may figure out what null stands for...

Chained method calls will keep you guessing

An exception will only tell you on what row the exception occurred. Consider the following row:

repository.getService(someObject.someMethod());

If you get an NPE and it points at this row, which one of repository and someObject was null?

Instead, checking these variables when you get them will at least point to a row where they are hopefully the only variable being handled. And, as mentioned before, even better if your error message contains the name of the variable or similar.

Errors when processing lots of input should give identifying information

Imagine that your program is processing an input file with thousands of rows and suddenly there's a NullPointerException. You look at the place and realize some input was incorrect... what input? You'll need more information about the row number, perhaps the column or even the whole row text to understand what row in that file needs fixing.

0

The reason for it, I do believe, is so that way you can have your code handle or divert the error to a way in which it can be properly communicated to the user what may have gone wrong. This is especially helpful since most users don't know Java and may not understand the error message, but if you explain it to them with a System.out.println, they may be able to fix it. Not only that, but if you want it to just cancel an operation if it happens to fail it can do so and return to something like the main menu from the operation or menu it was on. Lastly, when you are testing and want to know if something is running properly or failing, you would be able to tell what happened if you have it print something. Those are at least 3 uses I can currently think of.

Not the answer you're looking for? Browse other questions tagged or ask your own question.