Null check error message as “is null” or “was null

问题:

When doing null checks in Java code, and you throw IllegalArgumentExceptions for null values, what kind of message template do you use?

We tend to use something like this

public User getUser(String username){
   if (username == null){
     throw new IllegalArgumentException("username is null");   
   }
   // ...
}

What is better : "is null" or "was null", and why?

For me "is null" feels more natural.

回答1:

Since the Exception is thrown due to a failed precondition check, I think rather than simply stating a fact, you should state the requirement that was violated.

That is, instead of saying "username is null", say "username should not be null".

---

On using libraries for precondition checks

As a tip, you can use one of the many libraries designed to facilitate precondition checks. Many code in Guava uses com.google.common.base.Preconditions

Simple static methods to be called at the start of your own methods to verify correct arguments and state. This allows constructs such as

 if (count <= 0) {
   throw new IllegalArgumentException("must be positive: " + count);
 }

to be replaced with the more compact

 checkArgument(count > 0, "must be positive: %s", count);

More directly relevant here is that it has checkNotNull, which allows you to simply write:

  checkNotNull(username, "username should not be null");

Note how naturally the above code reads, with the detailed message explicitly stating the requirement that was violated.

The alternative of stating facts is more awkward:

 // Awkward!
 checkArgument(count > 0, "is negative or zero: %s", count);
 checkNotNull(username, "username is null");

Moreover, this is also potentially less useful, since the client may already be aware of the fact, and the exception doesn't help them figure out what the actual requirements are.

---

On IllegalArgumentException vs NullPointerException

While your original code throws IllegalArgumentException on null arguments, Guava's Preconditions.checkNotNull throws NullPointerException instead.

This is in accordance with the guideline set by the API:

NullPointerException: Applications should throw instances of this class to indicate other illegal uses of the null object.

Additionally, here's a quote from Effective Java 2nd Edition: Item 60: Favor the use of standard exceptions:

Arguably, all erroneous method invocations boil down to an illegal argument or illegal state, but other exceptions are standardly used for certain kinds of illegal arguments and states. If a caller passes null in some parameter for which null values are prohibited, convention dictates that NullPointerException be thrown rather than IllegalArgumentException.

回答2:

is null, since the argument is still null..

However, why not simply throw a NullPointerException without a message?

回答3:

I would suggest saying

  if (userName == null) {
     throw new IllegalArgumentException("username == null");
   }

as this is so fatal that a programmer must look at it anyway. Referring the offending code snippet in the exception message is the concisest thing I can imagine.

回答4:

I would be inclined to write this:

public User getUser(String username) {
   if (username.length() == 0) {
       throw new IllegalArgumentException("username is empty");   
   }
   // ...
}

This kills two birds with one stone. First, it detects the case where user name is an empty string, which (for the sake of argument) I'm assuming is an error. Second, if the parameter is null attempting to dispatch the length call will give a NullPointerException.

For the record, the expected exception to throw for an unexpected null is NullPointerException. If your main reason for not using it is that NPE's typically don't have a message, code it like this:

public User getUser(String username){
   if (username == null){
       throw new NullPointerException("username is null");   
   }
   if (username.length() == 0) {
       throw new IllegalArgumentException("username is empty");   
   }
   // ...
}

Why use NPE's here? Because NPE's almost always indicate a different kind of problem to a other kinds of argument validation error; e.g. a field or array cell that has not been initialized or an "optional" value that is not being handled properly.

Finally to the question:

What is better : "is null" or "was null", and why?

This is a matter of opinion, but I would write "is null".

• Because the message is reporting the state when the exception was thrown.
• Because it is conventional to do it that way.