Java constructor style: check parameters aren't null

Question

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.

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