Null Check Chain VS Catching Nullpointerexception

Null check chain vs catching NullPointerException

Catching NullPointerException is a really problematic thing to do since they can happen almost anywhere. It's very easy to get one from a bug, catch it by accident and continue as if everything is normal, thus hiding a real problem. It's so tricky to deal with so it's best to avoid altogether. (For example, think about auto-unboxing of a null Integer.)

I suggest that you use the Optional class instead. This is often the best approach when you want to work with values that are either present or absent.

Using that you could write your code like this:

public Optional<Integer> m(Ws wsObject) {
    return Optional.ofNullable(wsObject.getFoo()) // Here you get Optional.empty() if the Foo is null
        .map(f -> f.getBar()) // Here you transform the optional or get empty if the Bar is null
        .map(b -> b.getBaz())
        .map(b -> b.getInt());
        // Add this if you want to return null instead of an empty optional if any is null
        // .orElse(null);
        // Or this if you want to throw an exception instead
        // .orElseThrow(SomeApplicationException::new);
}

Why optional?

Using Optionals instead of null for values that might be absent makes that fact very visible and clear to readers, and the type system will make sure you don't accidentally forget about it.

You also get access to methods for working with such values more conveniently, like map and orElse.

Is absence valid or error?

But also think about if it is a valid result for the intermediate methods to return null or if that is a sign of an error. If it is always an error then it's probably better throw an exception than to return a special value, or for the intermediate methods themselves to throw an exception.

Maybe more optionals?

If on the other hand absent values from the intermediate methods are valid, maybe you can switch to Optionals for them also?

Then you could use them like this:

public Optional<Integer> mo(Ws wsObject) {
    return wsObject.getFoo()
        .flatMap(f -> f.getBar())
        .flatMap(b -> b.getBaz())
        .flatMap(b -> b.getInt());        
}

Why not optional?

The only reason I can think of for not using Optional is if this is in a really performance critical part of the code, and if garbage collection overhead turns out to be a problem. This is because a few Optional objects are allocated each time the code is executed, and the VM might not be able to optimize those away. In that case your original if-tests might be better.

How to avoid checking for null values in method chaining?

No, it is generally not good practice in Java to catch a NPE instead of null-checking your references.

You can use Optional for this kind of thing if you prefer:

if (Optional.ofNullable(country)
            .map(Country::getCity)
            .map(City::getSchool)
            .map(School::getStudent)
            .isPresent()) {
    isValid = true;
}

or simply

boolean isValid = Optional.ofNullable(country)
                          .map(Country::getCity)
                          .map(City::getSchool)
                          .map(School::getStudent)
                          .isPresent();

if that is all that isValid is supposed to be checking.

Check chains of get calls for null

The best way would be to avoid the chain. If you aren't familiar with the Law of Demeter (LoD), in my opinion you should. You've given a perfect example of a message chain that is overly intimate with classes that it has no business knowing anything about.

Law of Demeter: http://en.wikipedia.org/wiki/Law_of_Demeter

Null checks for a complex dereference chain in Java 8

I agree with both of you that Andrew Vershinin’s suggestion is the best we can do here and thus deserves to be posted as an answer.

nullableValue = Optional.ofNullable(bigRequest)
        .map(RequestCls::getData)
        .map(DataCls::getSamples)
        .filter(samples -> ! samples.isEmpty())
        .map(samples -> samples.get(0))
        .map(SampleCls::getValuableData)
        .map(ValDataCls::getValue)
        .orElse(null);

You will need to substitute the right class or interface names in the method references (or you may rewrite as lambdas if you prefer). Edit: If bigRequest itself cannot be null, the first method call should be just Optional.of(bigRequest).

It’s not the primarily intended use of Optional, but I find it OK. And better than items 1. and 3. (and 4.) from your question.

Handle Chained Method Calls avoiding NullPointerException - Which is the best way?

Use Optional:

return Optional.ofNullable(someMethod())
    .map(Meeting::getRoom)
    .map(Room::getProjector)
    .map(Projector::getModelName)
    .orElse("No Projector Exist");

As an aside, consider returning Optional or null from your method - having to compare your String to the special hardcoded value to detect the null case is going to get tiresome...

Sonarqube shows NullPointerException bug even after null check is present in Java

Theoretically, another thread might change the result of exception.getResponseHeaders() after you checked it's non null but before you call it a second time. This kind of check-then-act is not always safe.

The safer way to handle this is to assign to a local

var headers = exception.getResponseHeaders();
if (headers != null) {
    var contentType = headers.getContentType();
    if (contentType != null) {
        response.contentType(contentType);
    } 
}

I think this is a little more readable than your solution anyway.

Of course, if your object isn't mutated by multiple threads or is immutable, then this warning is a false positive which could be ignored.