Java constructor style: check parameters aren't null

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?

The second or the third.

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

For less verbosity use http://commons.apache.org/lang/api/org/apache/commons/lang/Validate.html">`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.

Old question; another new answer (already mentioned by another comment; but I think worth its own answer).

Java 7 added java.util.Objects.requireNonNull() to the APIs everybody can use. So checking all arguments for null boils down to a short list like:

this.arg1 = Objects.requireNonNull(arg1, "arg1 must not be null");
this.arg2 = Objects.requireNonNull(arg2, "arg2 must not be null");

Side notes:

• make sure to not reverse the two arguments - the second one is the message that will be used for the NPE which is thrown if the first argument is null (if you reverse them, well, then your check will never fail)
• another best practice: if possible, make all your class members final (so you can be sure: when some object has been created successfully, all its members are not null; and they won't change over time)

You can use one of the many libraries designed to facilitate precondition checks. Many code in Google 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 invocations 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

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.

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

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

Comparison of Ways to Check Preconditions in Java - Guava vs. Apache Commons vs. Spring Framework vs. Plain Java Asserts

public static void fooSpringFrameworkAssert(String name, int start, int end) {
		// Preconditions
		Assert.notNull(name, "Name must not be null");
		Assert.isTrue(start < end, "Start (" + start + ") must be smaller than end (" + end + ")");

		// Do something here ...
	}

	public static void fooApacheCommonsValidate(String name, int start, int end) {
		// Preconditions
		Validate.notNull(name, "Name must not be null");
		Validate.isTrue(start < end, "Start (%s) must be smaller than end (%s)", start, end);

		// Do something here ...
	}

	public static void fooGuavaPreconditions(String name, int start, int end) {
		// Preconditions
		Preconditions.checkNotNull(name, "Name must not be null");
		Preconditions.checkArgument(start < end, "Start (%s) must be smaller than end (%s)", start, end);

		// Do something here ...
	}

	public static void fooPlainJavaAsserts(String name, int start, int end) {
		// Preconditions
		assert null != name : "Name must not be null";
		assert start < end : "Start (" + start + ") must be smaller than end (" + end + ")";

		// Do something here ...
	}

this is summary of this article: http://www.sw-engineering-candies.com/blog-1/comparison-of-ways-to-check-preconditions-in-java

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

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.

I assume that you talk about the built in assert in Java. In my opinion it's not a really good idea to use it. Since it can be turned on/off using command line parameters. Therefore some says it is only acceptable to use in private methods.

My mentors are telling me not to re-invent the wheel! Their advice is to use libraries. They are (probably) well designed and tested. Of course it is your responsibility to make sure you grab a good-quality library.

Others are telling me that Enterprise ppl - in some terms - are wrong and you introduce more dependency - for simple tasks - than required. I can accept that point too... But here is my latest experience:

First I wrote my own private method to check null parameters. It's boring and redundant. I know I should put it into a Utility class. But why should I write it at the first place, when someone has already has done it? I can save time not writing unit test and design an existing stuff. Unless you want to exercise or learn I wouldn't recommend to do so.

I recently started to use google's guava and I find that - along with apache commons - once you start to use them, you won't use just for that one single method. You'll discover and use it more and more. At the end, this'll make your code shorter, more readable, more consistent and more maintainable.

BTW.: Depending on your aims I would go with 2 or 3 using one of the mentioned library above...