Reputation: 32233
Sometimes I face I must write a piece of code like this (usually it have more nested if and more complex structure but for the example is enought)
public void printIt(Object1 a){
if (a!=null){
SubObject b= a.getB();
if (b!=null){
SubObject2 c=b.getC();
if(c!=null){
c.print();
}
}
}
}
when I dont need to know what failed and if something is null do nothing, an approach could be
public void printIt(Object1 a){
try{
a.getB().getC().print();
}catch (NullPointerException e) {
}
}
Is there something wrong in this second form like performance or other kind of issues?
Upvotes: 34
Views: 35291
Reputation: 71
A code should never include exception handlers for unchecked exceptions. A null check should always be used for an object reference which has a chance of being null.
Upvotes: 0
Reputation: 16060
public void printIt(Object1 a){
if(a==null){
throw new IllegalArgumentException("a was null, but this is not allowed here."),
}
[...]
Fail fast and fail hard. If a shoud not be null, throw an Exception. This will make your code more stable and reliable.
So if I would have to decide between your a) and your b), I would choose a). But if a mustn't be null there, you would hide an error-situation.
Upvotes: 9
Reputation: 464
Using Java 8 optional:
Optional.ofNullable(a)
.map(Object1::getB)
.map(SubObject::getC)
.ifPresent(Object2::print);
Upvotes: 1
Reputation: 33749
If you migrate to Java 8 you can use Optional and Lambdas. First, you need to rewrite your classes to return Optional
of each type:
class Object1 {
private SubObject b;
Optional<SubObject> getB() {
return Optional.ofNullable(b);
}
}
class SubObject {
private SubObject2 c;
Optional<SubObject2> getC() {
return Optional.ofNullable(c);
}
}
class SubObject2 {
@Override
public String toString() {
return "to be printed";
}
}
Now, you can chain the calls without the risk of NullPointerExceptions in a concise way:
a.getB()
.flatMap(SubObject::getC)
.ifPresent(System.out::println);
See the Oracle's article Tired of Null Pointer Exceptions? Consider Using Java SE 8's Optional! for more information.
Upvotes: 0
Reputation: 160191
The exception version (similar to chains using Groovy's safe-navigation operator ?.
) makes it really easy to take the Law of Demeter (or as I call it, Demeter's Strongly-Worded Suggestion) and make it your plaything for the night.
Similarly, deeply-nested if
-statements leads to difficult-to-read code, and underneath it all, the same "violation" exists, and the cyclomatic complexity of such methods is high.
public void printIt(Object1 a) {
if (null == a) {
return;
}
SubObject b = a.getB();
if (null == b) {
return;
}
SubObject2 c = b.getC();
if (null == c) {
return;
}
c.print();
}
I'd rather see LAMs (Little Auxiliary Methods) in appropriate places that encapsulate the checks and pretty much eliminate the need for the question altogether.
Upvotes: 22
Reputation: 416
Definitely (a) but you should restructure the method to avoid nesting the if statements as mentioned in a previous answer. Exceptions are not the performance hit they once were but are still much slower than checking for null and should never be used for program flow control like this. If an object can be null you should check for it but if it is not allowed you should fail fast at the point you assign the object reference. In many circumstances you can have default implementations (empty list is a good example) to avoid nulls altogether which results in much cleaner code. Avoid nulls whenever you can.
Upvotes: 2
Reputation: 198033
Yes. The second version will have terrible performance.
Don't use exceptions for normal control flow. Effective Java item 57: use exceptions only for exceptional situations.
==UPDATE==
Even ignoring performance issues (exceptions are faster than they once were, according to my benchmark, but not nearly as fast as a simple if check), it's really code-smelly to use exceptions for standard program flow like this. JVM bytecode has special optimizations it can do for null checks, even in if
statements. The first code sample is vastly preferred.
Upvotes: 12
Reputation: 425003
The answer is use version A.
It is generally considered "bad design" to use Exceptions for flow control. Exceptions are for the "exceptional", especially NPEs which are totally avoidable using null checks. Further, using null checks, you can tell (ie log) which term is null (you won't know where the null is with your version B).
Note that performance is not an issue any more throwing exceptions (the stack trace for example is only built if you use it). It's a matter of clean code.
However, there are some case where using exceptions for flow control is unavoidable, for example the exceptions thrown from SimpleDateFormat.parse(), because there isn't a reasonable way to tell before making the call that your input is not parsable.
Upvotes: 2
Reputation: 8249
Using exceptions is always a bad idea in terms of performance, no matter how slow the mechanism used to be and is now. Whenever an exception is thrown, the full stack will be unrolled to create the stack trace. Thus, like Lois Wasserman said, you should not rely on them for (regular) program flow but for exceptional cases.
The nested ifs aren't the definition of beauty, but will give you the ability to print additional information like 'B is null' etc.
Upvotes: 2
Reputation: 28693
The wrongest part of the second version is that when a NPE happens inside the getB()
, getC()
it will be silently ignored. As already mentioned, exceptions are for exceptional cases.
Upvotes: 8