Java Constructor Style (Check parameters aren't null)

Question

What are the best practices if you have a class which accepts some parameters but none of them are allowed to be null?

The following is obvious but the exception is a little unspecific:

public class SomeClass
{
     public SomeClass(Object one, Object two)
     {
        if (one == null || two == null)
        {
            throw new IllegalArgumentException("Parameters can't be null");
        }
        //...
     }
}

Here the exceptions let you know which parameter is null, but the constructor is now pretty ugly:

public class SomeClass
{
     public SomeClass(Object one, Object two)
     {
        if (one == null)
        {
            throw new IllegalArgumentException("one can't be null");
        }           
        if (two == null)
        {
            throw new IllegalArgumentException("two can't be null");
        }
        //...
  }

Here the constructor is neater, but now the constructor code isn't really in the constructor:

public class SomeClass
{
     public SomeClass(Object one, Object two)
     {
        setOne(one);
        setTwo(two);
     }


     public void setOne(Object one)
     {
        if (one == null)
        {
            throw new IllegalArgumentException("one can't be null");
        }           
        //...
     }

     public void setTwo(Object two)
     {
        if (two == null)
        {
            throw new IllegalArgumentException("two can't be null");
        }
        //...
     }
  }

Which of these styles is best? Or is there an alternative which is more widely accepted?

Cheers,

Pete

Answer 1

You can simply have a method which takes all the constructor arguments that you need to validate. This method throws exception with specific message depending on which argument is not valid. Your constructor calls this method, and if it passes, it initialize values.

Answer 2

The second or the third.

Because it tells the user of your API what exactly went wrong.

For less verbosity use Validate.notNull(obj, message) from commons-lang. Thus your constructor will look like:

public SomeClass(Object one, Object two) {
    Validate.notNull(one, "one can't be null");
    Validate.notNull(two, "two can't be null");
    ...
}

Placing the check in the setter is also acceptable, with the same verbosity comment. If your setters also have the role of preserving object consistency, you can choose the third as well.

Answer 3

An alternative to throwing an unchecked exception would be the usage of assert. Otherwise I´d throw checked exceptions to make the caller aware of the fact, that the constructor will not work with illegal values.

The difference between your first two solutions - do you need a detailed error message, do you need to know which parameter failed or is it enough to know, that the instance couldn't have been created due to illegal arguments?

Note, that the second and third example can't report correctly that both parameters have been null.

BTW - I vote for a variation of (1):

if (one == null || two == null) {
    throw new IllegalArgumentException(
      String.format("Parameters can't be null: one=%s, two=%s", one, two));
}

Answer 4

I would have a utility method:

 public static <T> T checkNull(String message, T object) {
     if(object == null) {
       throw new NullPointerException(message);
     }
     return object;
  }

I would have it return the object so that you can use it in assignments like this:

 public Constructor(Object param) {
     this.param = checkNull("Param not allowed to be null", param);
 }

EDIT: Regarding the suggestions to use a third party library, the Google Preconditions in particular does the above even better than my code. However, if this is the only reasons to include the library in your project, I'd be hesitant. The method is too simple.

Answer 5

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);

It has checkNotNull that is used extensively within Guava. You can then write:

 import static com.google.common.base.Preconditions.checkNotNull;
 //...

 public SomeClass(Object one, Object two) {
     this.one = checkNotNull(one);
     this.two = checkNotNull(two, "two can't be null!");
     //...
 }

Most methods are overloaded to either take no error message, a fixed error message, or a templatized error message with varargs.

---

On IllegalArgumentException vs NullPointerException

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

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.

A NullPointerException isn't reserved for just when you access members of a null reference; it's pretty standard to throw them when an argument is null when that's an illegal value.

System.out.println("some string".split(null));
// throws NullPointerException

Answer 6

Annotations for static analysis are also useful, either in-addition-to or in-place-of the run-time checks.

FindBugs, for example, provides an @NonNull annotation.

public SomeClass( @NonNull Object one, @NonNull Object two) {

Answer 7

Apart from the answers given above which are all valid and reasonable, I think it's good to point out that maybe checking for null isn't necessary "good practice". (Assuming readers other than the OP might take the question as dogmatic)

From Misko Hevery blog on testability: To Assert or Not To Assert