Best practice with respect to NPE and multiple expressions on single line

Question

I'm wondering if it is an accepted practice or not to avoid multiple calls on the same line with respect to possible NPEs, and if so in what circumstances. For example:

anObj.doThatWith(myObj.getThis());

vs

Object o = myObj.getThis();
anObj.doThatWith(o);

The latter is more verbose, but if there is an NPE, you immediately know what is null. However, it also requires creating a name for the variable and more import statements.

So my questions around this are:

• Is this problem something worth designing around? Is it better to go for the first or second possibility?
• Is the creation of a variable name something that would have an effect performance-wise?
• Is there a proposal to change the exception message to be able to determine what object is null in future versions of Java ?

Answer 1

The Law of Demeter explicitly says not to do this at all.

Answer 2

Is this problem something worth designing around? Is it better to go for the first or second possibility?

IMO, no. Go for the version of the code that is most readable.

If you get an NPE that you cannot diagnose then modify the code as required. Alternatively, run it using the debugger and use breakpoints and single stepping to find out where the null pointer is coming from.

Is the creation of a variable name something that would have an effect performance-wise?

Adding an extra variable may increase the stack frame size, or may extend the time that some objects remain reachable. But both effects are unlikely to be significant.

Is there a proposal to change the exception message to be able to determine what object is null in future versions of Java ?

Not that I am aware of. Implementing such a feature would probably have significant performance downsides.

Answer 3

If you are sure that getThis() cannot return a null value, the first variant is ok. You can use contract annotations in your code to check such conditions. For instance Parasoft JTest uses an annotation like @post $result != null and flags all methods without the annotation that use the return value without checking.

If the method can return null your code should always use the second variant, and check the return value. Only you can decide what to do if the return value is null, it might be ok, or you might want to log an error:

Object o = getThis();

if (null == o) {
    log.error("mymethod: Could not retrieve this");
} else {
    o.doThat();
}

Answer 4

Personally I dislike the one-liner code "design pattern", so I side by all those who say to keep your code readable. Although I saw much worse lines of code in existing projects similar to this:

someMap.put(
     someObject.getSomeThing().getSomeOtherThing().getKey(),
     someObject.getSomeThing().getSomeOtherThing()) 

I think that no one would argue that this is not the way to write maintainable code.

As for using annotations - unfortunately not all developers use the same IDE and Eclipse users would not benefit from the @Nullable and @NotNull annotations. And without the IDE integration these do not have much benefit (apart from some extra documentation). However I do recommend the assert ability. While it only helps during run-time, it does help to find most NPE causes and has no performance effect, and makes the assumptions your code makes clearer.

Answer 5

If it were me I would change the code to your latter version but I would also add logging (maybe print) statements with a framework like log4j so if something did go wrong I could check the log files to see what was null.