Java constructor style: validation options are not null

Question

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

The following is obvious, but the exception is a little non-specific:

 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"); } //... } 

The constructor is more accurate here, but now the constructor code is actually not 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 better?

Or is there an alternative that is more widely accepted?

Answer 1

Second or third.

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

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

 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 comment commentary. If your setters also play the role of keeping objects consistent, you can also choose a third.

Answer 2

You can use one of many libraries designed to make it easy to verify prerequisites. A lot of code in Google Guava uses com.google.common.base.Preconditions

Simple static methods that you need to call at the beginning of your own methods to verify the correct arguments and state. This allows you to create designs such as

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

to replace it with a more compact

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

It has checkNotNull , which is widely used in Guava . Then you can 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 not accept the error message, neither the error message, nor the template error message with varargs.

---

On IllegalArgumentException vs NullPointerException

While your source code throws IllegalArgumentException on null arguments, Guava Preconditions.checkNotNull throws a NullPointerException instead.

Here is a quote from Effective Java 2nd Edition: Paragraph 60: Thank you for using standard exceptions:

It is possible that all erroneous method calls come down to an illegal argument or an illegal state, but other exceptions are standardly used for certain types of illegal arguments and states. If the caller skips null in some parameter for which NULL values ​​are prohibited, the convention dictates a NullPointerException , not an IllegalArgumentException .

A NullPointerException not reserved only when accessing null link elements; it is pretty standard to throw them when the argument is null , when this is an invalid value.

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

Answer 3

Old question; another new answer (already mentioned by another comment, but I believe its own answer is worth it).

Java 7 has added java.lang.Objects.requireNonNull() to the API that everyone can use. Thus, checking all arguments for null is reduced to a short list, for example:

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

Side notes:

• be sure not to flip two arguments - the second is the message that will be used for NPE, which is called if the first argument is null (if you change 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 was successfully created, all its members are not null and they will not change over time)

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 like it to return an object so that you can use it in assignments as follows:

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

EDITOR. Regarding offers to use a third-party library, Google’s premises, in particular, do it even better than my code. However, if these are the only reasons to include the library in your project, I would hesitate. The method is too simple.

Answer 5

Besides the above answers, which are valid and reasonable, I find it useful to note that perhaps checking a null value is not a mandatory “good practice”. (Assuming that readers, besides the OP, can pose the question as dogmatic)

From the Misko Hevery Testability Blog: To approve or not to approve

Answer 6

Comparison of methods for checking 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 a summary of this article: http://www.sw-engineering-candies.com/blog-1/comparison-of-ways-to-check-preconditions-in-java

Answer 7

An alternative to throwing an unchecked exception is to use assert . Otherwise, I would choose checked exceptions to notify the fact that the constructor will not work with invalid 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 for him to know that the instance could not be created due to illegal arguments?

Please note that the second and third examples cannot correctly report that both parameters are null.

By the way - I vote for option (1):

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

Answer 8

Static analysis annotations are also useful, either in addition to or in place of run-time checks.

FindBugs, for example, provides the @NonNull annotation.

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

Answer 9

You can simply have a method that takes all the constructor arguments that need to be checked. This method throws an exception with a specific message depending on which argument is invalid. Your constructor calls this method, and if it passes, it initializes the values.

Answer 10

I assume that you are talking about the built-in assert in Java. In my opinion, this is not a good idea. Since it can be turned on / off using command line options. Therefore, some say that this is acceptable only for private methods.

My mentors tell me not to reinvent the wheel! Their advice is to use libraries. They are (probably) well designed and tested. Of course, you are responsible for capturing a good quality library.

Others tell me that Enterprise ppl - in some terms - is wrong, and you introduce more dependency - for simple tasks - than you need. I can also take this moment ... But here is my last experience:

First I wrote my own private method for checking null parameters. It is boring and boring. I know that I have to put it in the Utility class. But why should I write this in the first place, when someone has already done this? I can save time by not writing down unit test and not creating existing material. If you do not want to train or study, I would not recommend doing this.

I recently started using google guava, and I found that with apache commons - as soon as you start using them, you will not use only one single method. You will discover and use it more and more. In the end, this will make your code shorter, more readable, more consistent and more convenient.

By the way: Depending on your goals, I would go with 2 or 3 using one of the above libraries above ...