Search code examples
javanullpointerexceptionthrow

Should I write null checks for all of my methods?


I'm creating my own library like Apache Commons DigestUtils for learning purposes.

Inside of my update() method I've written a simple if (something == null) throw new Exception as null checking.

/**
 * Performs a digest update for the {@code String} (converted to bytes using the
 * UTF-8 standard charsets) with a specific {@code MessageDigest} and returns
 * the final digest data.
 *
 *
 * @param messageDigest the {@code MessageDigest} with a specific algorithm to
 *                      process the digest.
 *
 * @param data          the data to digest.
 *
 * @return the {@code MessageDigest} with the processed update for the digest.
 *
 * @throws IllegalArgumentException if {@code messageDigest} is {@code null}.
 *
 * @throws IllegalArgumentException if {@code data} is {@code null}.
 */
public static MessageDigest update(final MessageDigest messageDigest, final String data) {
    if (messageDigest == null)
        throw new IllegalArgumentException("messageDigest cannot be null");
    if (data == null)
        throw new IllegalArgumentException("data cannot be null");

    messageDigest.update(data.getBytes(StandardCharsets.UTF_8));

    return messageDigest;
}

Now I'm not sure if I should write the same checks in other methods that must call the update() method, or only write it in the comments that the method throws the corresponding exceptions.

public static byte[] digest(final MessageDigest messageDigest, final byte[] data) {
    return update(messageDigest, data).digest();
}

or

public static byte[] digest(final MessageDigest messageDigest, final byte[] data) {
    if (messageDigest == null)
        throw new IllegalArgumentException("messageDigest cannot be null");
    if (data == null)
        throw new IllegalArgumentException("data cannot be null");

    return update(messageDigest, data).digest();
}

Note that digest() method calls update() method.

What is the best practice?


Solution

  • 1.: There is the function Objects.requireNonNull, which is exactly for this purpose. It throws an exception if the given argument equals null.

    if (messageDigest == null)
            throw new IllegalArgumentException("messageDigest cannot be null");
    

    should be just

    Objects.requireNonNull(messageDigest, "messageDigest cannot be null");
    

    2.: The best practice to throw an exception because of null is to throw it where the null shouldn't occur in the first place. This is the motivation behind the Fail Fast Principle.

    When a null reference occurs where it should not be, you want to throw an exception, or handle it, at exactly that spot. You don't want to pass the null referece to other methods/functions. Otherwise you would have a harder time debugging your code when the passed null reference causes problems in your program. This is because it won't be clear where the null reference occured in the first place; but what actually caused it is what you want to fix/handle. An example:

    public class Demo {
        public void a(Object obj) {
            b(obj);
        }
        private void b(Object obj) {
            c(obj);
        }
        private void c(Object obj) {
            // Calculation using obj.
        }
    }
    

    If it's clear that a should never get a null reference passed, this is the correct place to check for it.

    public void a(Object obj) {
        Objects.requireNonNull(obj); // Throw exception when null.
        b(obj);
    }
    

    It's bad when you do a null check in every single method because the fact that obj shouldn't be null counts for all those methods, thus it's enough to place the null check where the null shouldn't occur in the first place, i.e. a. The program shall fail fast.

    It would be even worse to put the null check in c because c should logically never receive a null reference from b, and b shouldn't receive one from a. If the null reference causes problems in c, you will have a harder time finding out if null occured in b or a or the code that called a. You want to know that because you want to fix the problem of the occuring null reference, not the problems caused by it. Of course, sometimes you can't fix the occurrence itself, but you can try to get as close as possible to the occurrence and reduce collateral "damage" done by it.

    There is no real goto way to place null checks. You have to place them at the right places to make the program fail fast on occurring invalid objects. It's up to you to decide where the best places for null checks are because it all depends on the exact case.

    I hope this gives you a good idea about how to approach this.