Which is a better practice for if-else condition?

Question

Which one of these two options are better?

if(obj.getType!=null && obj.getData()!=null)
{
  // Do something
}
elseif(obj.getData==null)
{
  // Do other thing
}

*OR*

if(obj.getType!=null)
{  
  if(obj.getData!=null)
  {
    // Do something
  }
  else
 {
   // Do other thing
 }
}

The major point of doubt is why to do null check for getData twice in the first way? The second option does it only once.

It's a matter of opinion. Personally, the complexity of the requirements dictate which style I might use — MadProgrammer, Aug 11 '14 at 00:43
The second one seems better to me as it is more readable and will be more easily maintainable. But as MadProgramer said, it is a matter of opinion. — nstCactus, Aug 11 '14 at 00:45
The two if-else statements have different behaviour. When obj.getType==null and obj.getData==null. — hk6279, Aug 11 '14 at 00:58
Well, since the first one doesn't compile, I'd go with the second one... — Makoto, Aug 11 '14 at 01:12

score 2 · Answer 1 · edited Aug 11 '14 at 01:03

2

There is a slight difference between the two options. In the first option, the part elseif(obj.getData==null) will be executed whether object.getType is null or not. But, in the second option, the block will be executed only if object.getType is not null. So, I think it depends upon your requirement.

edited Aug 11 '14 at 01:03

jdphenix

15,022
3
41
74

answered Aug 11 '14 at 01:00

umet

29
3

Nice catch +1, and yes, it definitely depends on what you're trying to accomplish. – jdphenix Aug 11 '14 at 01:04
Agreed. But if we look just into performance sake, which one would be better? – Vivin Aug 11 '14 at 02:21

jdphenix · Answer 2 · 2014-08-11T05:31:35.180

It depends on what you need.

Consider you have a requirement that stipulates you collect 2 data points that represent values that are greater than 0, and you have to give feedback on if and which values are invalid.

public Data(int x, int y) 
{
    if (x <= 0 || y <= 0) { 
         throw new IllegalArgumentException("Invalid data provided, got (x: %d, y:%d)", x, y); 
    }
}

Note that checking for null and then checking for a condition is a fairly normal pattern called a null guard.

public void setValue(SomeValue o) { 
    if (o == null || o.Value <= 0) { 
        // code to handle bad value
    }
}

On the other hand, if you have a set of conditions that need to be checked in order, and fail out if one isn't met, you can (not recommended) use chained ifs like you have:

public void someOperation() 
{ 
    if (condition) 
    { 
        if (condition2) 
        {
            if (condition3) 
            { 
                // operation here
            } 
        }
    }
}

Note that although you don't explicitly mention something other than control statements, if you have a situation similar to the preceeding example, I would strongly recommend refactoring the condition checks by inverting them, and if the checks are complicated, refactor them out to methods returning boolean.

public void someOperation() 
{ 
    if (!condition) 
    {
        // code to fail out
    }

    if (!condition2) 
    {
        // code to fail out
    }

    if (!complexCondition())
    {
        // code to fail out
    }

    // operation here
}

Thanks. But if we look just into performance sake, which one would be better? — Vivin, Aug 11 '14 at 02:21
@jdphenix Your guardIf() method's condition are wrong. It should be x<=0 and y and z for second and third condition. (Although the performance would not be affected) — hk6279, Aug 11 '14 at 05:02
Sorry, but your benchmark results are worthless. See e.g. [here](http://code.google.com/p/caliper/wiki/JavaMicrobenchmarks). Btw., any Java benchmark taking less than a second is crap (google for warm-up). — maaartinus, Aug 11 '14 at 05:30

Sagi · Answer 3 · 2014-08-11T01:37:11.403

Null check for getData isn't always done twice in the first example. It is only ensured to get done no matter what. The && operator checks if left condition is true (if obj.getType != null), and only if it is, does it proceed on with checking the second one (if obj.getData() != null). && and || are lazy operators. If they can determine the value of whole boolean statement after checking the first condition, then they do not go onto examining the others - unlike & or |.

Second version doesn't ever check for obj.getData value unless the outer condition is met. If it is not, you never get to reach the second block.

Which is a better practice for if-else condition?

3 Answers3

Linked