java Nested If or single if

Question

I have a basic doubt regarding the execution of the following code block (Sample):

String version = computer.getSoundcard().getUSB().getVersion();

Which might throw NullPointerException if Soundcard isn't there.

So I have ,

Option 1 :

if(computer!=null && 
        computer.getSoundCard() !=null && 
                 computer.getSoundCard().getUSB()!=null) {
   version = computer.getSoundcard().getUSB().getVersion();
}

Option 2 :

if(computer !=null){
   SoundCard sc = computer.getSoundCard();
   if(sc!=null){
      USB usb = sc.getUSB();
      if(usb!=null){
         version = usb.getVersion();
      }
   }
}

As per my understanding the Option 1 will have extra overhead as it has to evaluate the same expression multiple times like computer.getSoundCard() 3 times, computer.getSoundCard().getUSB() 2 times.

Is my understanding correct ?

EDIT 1: Changed Option 2 from

version = computer.getSoundcard().getUSB().getVersion();

Can you to use Java 8 on your project... If yes, look at this: docs.oracle.com/javase/8/docs/api/java/util/Optional.html — Joao Polo
– Joao Polo, Commented Sep 18, 2015 at 2:51
They are both good. I'd usually go with option 1 because it's more concise. Option 2 might be better if 1) you change to version = usb.getVersion(), and 2) any of the getter methods might be slow. — Andreas
– Andreas, Commented Sep 18, 2015 at 2:54
and... java compiler will to optimize that kind of tasks... try to look at generated jvm code for your code... it's more optimized than you thought. — Joao Polo
– Joao Polo, Commented Sep 18, 2015 at 2:54
Assuming these methods return reference to objects (or null) - the performance penalty for calling the same getter twice is negligible. That said, if you're performing heavy computation inside any of these methods - that's indeed something to consider. — Nir Alfasi
– Nir Alfasi, Commented Sep 18, 2015 at 2:57
@Deva Getter methods are usually just returning a field, so cost is nil. Usually. — Andreas
– Andreas, Commented Sep 18, 2015 at 3:07

shihpeng · Accepted Answer · 2015-09-18 09:35:34Z

24

A better approach is to extract this USB-version-getting code into another single method, say getComputerUsbVersion(), then flatten the super long if or the nested if-else block into several simple if blocks:

public String getComputerUsbVersion(Computer computer) {

    if (computer == null)  return null; 

    SoundCard soundCard = computer.getSoundCard();
    if (soundCard == null) return null; 

    USB usb = soundCard.getUSB()
    if (usb == null) return null;

    return usb.getVersion();
}

As you can see, the code is much cleaner and easy to understand, the super long if condition or the nested if-else block is also avoided. You can even add more condition checking code to this method later on very easily.

edited Sep 18, 2015 at 9:35

answered Sep 18, 2015 at 3:09

shihpeng

5,4017 gold badges40 silver badges65 bronze badges

Sign up to request clarification or add additional context in comments.

2 Comments

Joao Polo Over a year ago

I agree because this form will encapsulate the logic to get the usb version. Well, the function name could be more specific, like findSoundCardUsbVersion... "find", not "get", because get presumes with it exists... and there are many points of return on the method.

Fabio Filippi Over a year ago

Even better would be to raise an exception (and log an error) when null is found.

Sergey Kalinichenko · Accepted Answer · 2015-09-18 17:21:46Z

17

As per my understanding the Option 1 will have extra overhead as it has to evaluate the same expression multiple times

Yes, these calls would be made multiple times. However, you can shorten it if you make assignments as part of your conditional, like this:

SoundCard sc;
USB usb;
if(computer != null && (sc = computer.getSoundCard()) != null && (usb = sc.getUSB()) != null) {
    version = usb.getVersion();
}

Note that references to sc and usb inside the expression and inside the conditional are safe, because && evaluation is guaranteed to stop upon reaching the first false in the chain.

edited Sep 18, 2015 at 17:21

answered Sep 18, 2015 at 2:53

Sergey Kalinichenko

729k85 gold badges1.2k silver badges1.6k bronze badges

1 Comment

lschuetze Over a year ago

Just to add: You should realize that you are defining those variables sc and usb in a much wider scope as you might need. I would not want to read an if expression like this in my source code.

Collectives™ on Stack Overflow

java Nested If or single if

2 Answers 2

2 Comments

1 Comment

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

2 Comments

1 Comment

Your Answer

Sign up or log in

Post as a guest

Related