Addev
Addev

Reputation: 32233

try/catch vs null check in java

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

Answers (10)

Noroi
Noroi

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

Christian Kuetbach
Christian Kuetbach

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

seenimurugan
seenimurugan

Reputation: 464

Using Java 8 optional:

Optional.ofNullable(a)
            .map(Object1::getB)
            .map(SubObject::getC)
            .ifPresent(Object2::print);

Upvotes: 1

matsev
matsev

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

Dave Newton
Dave Newton

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

nicktalbot
nicktalbot

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

Louis Wasserman
Louis Wasserman

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

Bohemian
Bohemian

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

sunside
sunside

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

khachik
khachik

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

Related Questions