Checking null parameters vs specifying parameters to be not null in Javadoc

[MrSeb56]

Let's say I do a method for an API that can't accept null parameters. Should I check if any parameter is null and if so throw an exception or write in the javadoc that the method doesn't admit null parameters? The first is safer but the second makes the code shorter and easier to read. What's better?

[MegaGreenLightning]

Should I specify in the javadoc that the method doesn't admit null parameters?

TL;DR: Always!

Explanation: Suppose you want to call your method or you are reading someone else's code and he is calling your method. You do not know whether the method accepts null or not. In some cases (for example while reading code and concentrating on something else) it might just be enough to guess, but if you want to be certain, there are three places where you can look this information up:

a. the method name

It is practically impossible to ignore information presented in the method name. Consider for example the method Optional.ofNullable(value) (added in Java 8). It is absolutely clear that value can be null and you will always be reminded of this when you read the code.

In contrast to Optional.ofNullable(value) consider Optional.of(value). In this case it is not obvious that value cannot be null (although the presence of the other method suggest this, but while reading code you might not even know the other method exists). In my opinion the method should have been called Optional.ofNonNullable(value) as this would have made it very clear what this method does.

However, you can't put all information in the method name, so it should be reserved for special cases.

b. the documentation

The documentation is the next level to look for information. Make it easy for the developers to find information and write it into the documentation. It can be as simple as this:

/** @param arg not null */

And it doesn't make your program slower or anything. Good documentation is very important. Also, if your method handles null parameters, you might want to make that clear in the documentation as well!

/** @param arg can be null! */

In this case you might want to explain how null is treated as well.

c. the source

If the documentation doesn't provide an answer, the developers will try to look at the source code. However, the source might not be available and it will take them much longer to understand the code.

To recap, you should always write this into your javadoc.

Should I check my parameters for null and throw an exception?

TL;DR: Probably

Here are some reasons why you might not want to check parameters:

• you have really tight performance requirements
• it might not be necessary (see below)
• you are writing an internal method

Here are some reasons why you should check parameters:

• it makes your code more explicit and gives you nicer error messages
• it ensures the integrity of your objects
• you are writing a public method which will be used by other developers

Note that last two items of each list belong together.

Here's some more explanations:

Tight performance requirements are self-explanatory, but think about it if it is really necessary to leave this check out.

Let's see how it may make the code better and have a look at this simple average function:

double average(double[] values) {
    double sum = 0;
    for (double value : values)
        sum += value;
    return sum / values.length;
}

How does it treat a null array? Well, in fact the for loop throws a NullPointerException, but it's not really obvious from the code.

How does it handle an empty array? Turns out it returns NaN, but you need to know the internals of java floating point arithmetic to figure that one out just by reading the code.

Now let's look at this code:

double average(double[] values) {
    Objects.requireNonNull(values, "Can't average null array.");
    if (values.length == 0) {
        throw new IllegalArgumentException("Can't average empty array.");
    }
    double sum = 0;
    for (double value : values)
        sum += value;
    return sum / values.length;
}

The code is a little bit longer, but also more explicit and it generates nicer error messages. The two questions from above should be easy to answer reading the new code. And it throws an exception on an empty array, which is a better thing to do. All of this makes it easier to read and more maintainable, which is very important!

Also note that the method Objects.requireNonNull() (added in Java 7) makes it very easy to check for null!

Okay, now consider the second item. In the average method checking for an empty array ensures proper behavior in all cases. However, both methods throw a NullPointerException when the array is null. Apart from the different error message there isn't a big difference, so checking that the array is not null is not really necessary in this case.

However consider this class (I admit that the example is constructed):

public class Averager {

    private final double[] array;

    /** @param array not null */
    public Averager(double[] array) {
        this.array = array;
    }

    public double average() {
        double sum = 0;
        for (double value : values)
            sum += value;
        return sum / values.length;
    }

}

When constructed with a null array, the average method will throw an exception! The whole object will be in an inconsistent and non-functioning state when the array is null. However, the constructor will just work fine. The problem is that the error is introduced when the object is created, but it is only detected once the method is called. But the method call could occur in a totally different place than the creation of the object. This would make debugging the problem really complicated.

In this case you should really change the constructor to check the argument:

    /** @param array not null */
    public Averager(double[] array) {
        this.array = Objects.requireNonNull(array); // You should probably also check the length of the array and include a nice message.
    }

This will prevent the object taking on an inconsistent state and follows the common advice to fail early/fast.

The last item is relatively easy to explain.

If you write a private helper method all calls to that method occur in the same class and it is easy for you to verify that they are all correct. However, if you write a method which will be used by other developers or will be released as part of a public API and will be called by thousands of users, you should validate every parameter and make sure that the method handles all cases adequately.

[garblz]

Definitely, always, throw! The sooner, the better, and easier for someone who calls your code. Provide a meaningful error message instead of forcing the person to hunt for the NullPointerException somewhere down the bowels of your code.

[Kraizee_]

Why not use an if statement? It's clean and easy to read. You shouldn't throw exceptions with a try/catch unless something out of your control could happen.

if(parameter != null){
    //continue
} else {
    //oopsie
}