Null check error message as "is null" or "was null"

Question

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.

Answer 1

is null, since the argument is still null..

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

Answer 2

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 invokations boil down to an illegal argument or an illegal state, but other exceptions are standardly used for certain kinds of illegal argument and states. If a caller passes null in some parameter for which null values are prohibited, convention dictates NullPointerException be thrown rather than IllegalArgumentException.

Answer 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.

Answer 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 us the NullPointerException.