Assert for null check

Question

It seems widely accepted that assert statements should be reserved for testing and disabled in production because errors should have been resolved by then, and enabling assertions affects performance. However surely the same could be said of null checks with if statements. Why is this code considered good for production

if(x != null) {
    x.setId(idx);
    if (y != null) {
        if (y.id == x.id) {
            x.doSth();
        }
    } else {
        //handle error
    }
} else {
    //handle error
}

but this code isn't? (Assuming assertions are enabled)

try {
    assert(x != null); 
    x.setId(idx);
    assert(y != null);
    if (y.id == x.id) {
        x.doSth();
    }
} catch (AssertionError e) {
    //handle error
}

I understand using an if statement when it's expected that a variable may not be initialized. However when it's for defensive coding, assert seems more elegant and readable.

I also tested performance for each method with this:

public class AssertTest {

    static final int LOOPS = 10000000;

    public static void main(String[] args) {

            String testStr = "";

            long startNotEqualsTest = System.currentTimeMillis();
            for (int i = 0; i < LOOPS; i++) {
                if (testStr != null) {
                    testStr = System.currentTimeMillis() + "";
                } else {
                    throw new RuntimeException("We're doomed");
                }
            }
            long notEqualsTestDuration = System.currentTimeMillis() - startNotEqualsTest;

            testStr = "";

            long startAssertTest = System.currentTimeMillis();      
            for (int i = 0; i < LOOPS; i++) {
                try {
                    assert(testStr != null);
                    testStr = System.currentTimeMillis() + "";
                } catch (AssertionError e) {
                    throw new RuntimeException("We're doomed");
                }
            }
            long assertTestDuration = System.currentTimeMillis() - startAssertTest;

            System.out.println("Duration with if : " + notEqualsTestDuration + "ms");
            System.out.println("Duration with assert : " + assertTestDuration + "ms");  
    }
}

Timing is about the same for both methods:

Duration with if : 1234ms
Duration with assert : 1313ms

Disabling assertions makes very little difference:

Duration with if : 1391ms
Duration with assert : 1375ms

Have I missed any compelling reasons why null checks with if conditions are preferable to assert?

Answer 1

Ask yourself how you would handle the null cases? What do you do if an object is null that should not be. In most of the cases it absolutely ok to not handle it, at all. Just let it produce a NullPointerException sometime later in the execution. It is very likely to be a programming error, anyway, so it's ok to be caught by an ExceptionHandler eventually writing it to the logs.

Of course, there are cases when you need to react on null objects. if-else is made for such cases. It's easy, self-explaining, every programmer knows that construct, so why not using it. assert in production is discouraged, anyway. See this post on SO, for instance. And, for my taste, it's quite cumbersome with the try/catch block.

There are other options, as well. For instance, DavidW suggested to use annotations, which is perfectly fine, as long as you make sure that these annotations are interpreted (can be an issue when migrating code, for example).

Another approach are Validator classes. For instance, the Apache Commons library has a Validate class that checks for certain conditions and would throw an appropriate exception if the condition is not fullfilled. Of course, you can write your own Validators that will throw your custom exceptions, as well. You'll end up with a short concise one-liner like

Validate.notNull(myObj) 
Validate.hasEmailCorrectSyntax("foo@bar.com");

Also take a look at Java's Object.requireNotNull

Answer 2

Using assert at all has to presume that the -ea flag is enabled, and in many instances...it isn't. The assert will accomplish the same thing as the null checks, with the clear side-effect that if one doesn't have their environment configured in a particular way, the code will not function as they expect.

For that reason, it makes sense to eschew assert and leverage the null checks where necessary. More concisely, if one can, it's preferable to wrap everything that could be null in an Optional instance and operate on it using ifPresent (or orElseThrow since you seem to want to indicate an error if those values are null) instead.

Answer 3

In my experience, if x or y being null is, in fact, an error then (instead of your first example) what will be coded is:

void someFunction(AnObject x, AnObject y) {
    if (x == null || y == null) {
        throw new IllegalStateException("Contract violation: x and y must be non-null");
    }
    // rest of method can be run without null checks.
}

As I've noted in some of the discussion below, if you know up front that certain parameter values are breaking (like null or NaN in a float/double) then it makes a lot of sense to fail fast. It helps callers isolate the problem better than if a null value gets put in a Map and the NPE gets thrown on another thread, for instance.

(Note: edited to remove a contentious suggestion to use annotations.)