Are a bunch of if statements inefficient or bad in any way?

[Maximilian]

Is there any advantage to if-else statements or nesting if's when a group of say 5 or so if's will do the same job and likely be more readable?

 

[Pia]

It depends, practical example would help.

The fewer paths execution can take through the code, the better.
As a corollary of that, the less conditional statements there are in the code, the better.
The less nesting, the better.
In a situation where the control flow is such that you can naturally write a bunch of if-elses, writing a bunch of ifs is going to be less readable. (And less efficient, which won't matter for 99% of code.)

 

[Oyster]

// Bad Juju... removed

 

[Cogman]

I don't see how an "else" makes something less readable.

The first issue is that a group of if statements are not the same as an if else statement. They can be made the same, but at the cost of readability (You would need to either use gotos or have a variable that stops the following if statements from being checked.)

The next is speed. With a group of if statements, every if statement is checked (unless you use a goto or something similar) checking is NOT free, so yes, there would be a slight performance decrease.

You get the highest speed by putting the most common condition as the first check and ignoring the rest. The prevents needless checking in the rest of the else statements.

The less nesting, the better.

Not always true. Lets say you are doing collision detection in which you want to know if a bullet passes through one of the planes of a 3d model. One of the simplest ways to do this check is to surround the model with a sphere and then check if the bullet is inside the sphere. This is a VERY quick calculation. Running this check first and then the real collision detection if it fails will save a ton of processing time.

Nesting done wrong can be slow, but done right it can really boost the codes speed. (The right way is to use simple calculations to check if complex calculations are necessarily).

Then, of course, there are times when it unavoidable, but we aren't really talking about those.

 

[Pia]

...

I would expect any halfway sane compiler to compile your two logically equivalent code examples identically. The latter is only better because it has less nesting and is therefore easier for a human to read - it's not an optimization.

 

[Oyster]

I would expect any halfway sane compiler to compile your two logically equivalent code examples identically. The latter is only better because it has less nesting and is therefore easier for a human to read - it's not an optimization.

10-4. Didn't see what the hell I was typing. Sunday morning blues .

 

[Pia]

Not always true. Lets say you are doing collision detection in which you want to know if a bullet passes through one of the planes of a 3d model. One of the simplest ways to do this check is to surround the model with a sphere and then check if the bullet is inside the sphere. This is a VERY quick calculation. Running this check first and then the real collision detection if it fails will save a ton of processing time.

I was talking about how to arrange logically equivalent code, of course. I'll use your example to illustrate:

Spoiler

// More nesting - usually bad

if (quickCheck())
{
    for (...)
    {
        while (...)
        {
            ...
        }
        ...
    }
}
else
    return NOHIT;

// Less nesting - usually good

if (!quickCheck())
    return NOHIT;

for (...)
{
    while (...)
    {
        ...
    }
    ...
}

Nesting done wrong can be slow, but done right it can really boost the codes speed. (The right way is to use simple calculations to check if complex calculations are necessarily).

The choice of nested ifs versus else ifs versus ifs with several clauses connected with logical operators has almost nothing to do with performance.

 

[degibson]

Optimize your control structures for readability. Have a really, really good reason if you decide to do one if before another for "performance reasons", and call it out explicitly if you do so, e.g.,

// Do this check first, since we can avoid I/O
if (0 == x) {
  ...
} else if (ThingThatMightDoBlockingIO()) {
  ..
}

... but if all the predicates are simple, the organization of the control flow itself (nested if/else, successive if, etc.) often doesn't matter. So much so that if you have to ask, it probably doesn't matter.

 

[alkemyst]

you doing ColdFusion?

 

[AyashiKaibutsu]

Optimize your control structures for readability. Have a really, really good reason if you decide to do one if before another for "performance reasons", and call it out explicitly if you do so, e.g.,

// Do this check first, since we can avoid I/O
if (0 == x) {
  ...
} else if (ThingThatMightDoBlockingIO()) {
  ..
}

... but if all the predicates are simple, the organization of the control flow itself (nested if/else, successive if, etc.) often doesn't matter. So much so that if you have to ask, it probably doesn't matter.

This. FML I've had to deal with code where do stuff like that but even more complicated without comments and I had to hunt down everywhere were x was referenced to know for sure what it'd do (and it was a variable passed from another class). The worst part was it ended up being just fancy code to only execute some chunk on the first pass of a loop that they could have easily just pulled out of the loop.

 

[KIAman]

Depending on the language, always strive to optimize for readability. Most compilers are smart enough to optimize conditions and branches that favor performance when reduced to bytcode or assembly.

Even then, there are some compilers (cough delphi) that will always do complete boolean evaluations and tree evaluations by default unless you turn it off, so... just be mindful of your compiler settings.

 

[wayliff]

Off topic a little bit...

For readability I would prefer to code this in the following manner...
I am ok with code that is a bit more wordy but anyone, I think, can read it.

This is my preference and in no way I'm saying it is the better way.

I prefer boolean checks to have their variable since long method names can get confusing inside if statements.

I also prefer to include comments inside the if statement, if possible. Some programmers might copy and paste a block of code and forget to copy comments. In this manner, the comments are forced.

I also prefer code that has a single return path. Multiple return paths are a pain when debugging.

bool blockingIO = ThingThatMightDoBlockingIO(); 
int value = 0;

if (0 == x) 
{ // Do this check first, since we can avoid I/O   
     value = 1; 
} 
else if (blockingIO)
{   
     value = 2; 
}

return value;

 

[wayliff]

here is an interesting article from coding horror...
http://www.codinghorror.com/blog/2006/01/flattening-arrow-code.html

 

[iCyborg]

Off topic a little bit...

For readability I would prefer to code this in the following manner...
I am ok with code that is a bit more wordy but anyone, I think, can read it.

This is my preference and in no way I'm saying it is the better way.

I prefer boolean checks to have their variable since long method names can get confusing inside if statements.

I also prefer to include comments inside the if statement, if possible. Some programmers might copy and paste a block of code and forget to copy comments. In this manner, the comments are forced.

I also prefer code that has a single return path. Multiple return paths are a pain when debugging.

bool blockingIO = ThingThatMightDoBlockingIO(); 
int value = 0;

if (0 == x) 
{ // Do this check first, since we can avoid I/O   
     value = 1; 
} 
else if (blockingIO)
{   
     value = 2; 
}

return value;

The things I don't like about this:
1. Your code doesn't avoid possibly unnecessary I/O blocking check. It's the first thing it does! Unless the compiler is really smart and can figure out that it can postpone setting blockingIO.
2. The norm is to put a comment before the code. If the first line wasn't "value=1" but some other if-statement, it could be easily confusing to know which one are you commenting.
3. In the light of 1, the comment isn't even correct.
4. If a function is named properly and used only once, assigning a result to boolean doesn't do much for readibility to me.

 

[wayliff]

The things I don't like about this:
1. Your code doesn't avoid possibly unnecessary I/O blocking check. It's the first thing it does! Unless the compiler is really smart and can figure out that it can postpone setting blockingIO.
2. The norm is to put a comment before the code. If the first line wasn't "value=1" but some other if-statement, it could be easily confusing to know which one are you commenting.
3. In the light of 1, the comment isn't even correct.
4. If a function is named properly and used only once, assigning a result to boolean doesn't do much for readibility to me.

I don't disagree with your comments as it is a bad illustration. Regarding # 4 ... it is quite subjective.

I did not take any time to make it better according how to I like to code...but let's tweak it a bit.

bool blockingIO = ThingThatMightDoBlockingIO();  
int value = 0;  

if (blockingIO) 
{ // Comment here       
     value = 1;  
}
else if (x == 0)
{
     value = 2;
}
else
{
     value = 3;
}

return value;

 

[Modelworks]

You are better off using switch/case if you can rather than a bunch of if statements.
The output from switch/case is often cleaner assembly than a bunch of if.

 

[sourceninja]

You are better off using switch/case if you can rather than a bunch of if statements.
The output from switch/case is often cleaner assembly than a bunch of if.

One thing I find strange is that in some scripting languages like php using a bunch of if statements is actually faster than using a switch statement. (Providing your bunch of if statements are not making a classic mistake like calling a function)

I still don't understand that.

Also in terms of if statement blunders. I get interns that write this a lot.

(really quick and dirty example)
$value = 5
if($someobject->method($value) == 1)
...
if($someobject->method($value) == 2)
....
if($someobject->method($value) == 3)
...

where $someobject->method is some fairly complicated calculation (like a sql query with a long runtime).

They seem kinda dumbfounded as to why you would want to only run the method once and just compare the result 3 times.

 

[Modelworks]

One thing I find strange is that in some scripting languages like php using a bunch of if statements is actually faster than using a switch statement. (Providing your bunch of if statements are making a classic mistake like calling a function)

I still don't understand that.


where $someobject->method is some fairly complicated calculation (like a sql query with a long runtime).

They seem kinda dumbfounded as to why you would want to only run the method once and just compare the result 3 times.

I do a lot of embedded programming where you have to count clock cycles for instructions to execute or the code will not execute correctly and a lot of the compilers I use will not let you do calculations inside a conditional statement. You can do something simple like x<0 or x!=0 but you cannot do x/4<2 . The reason is that it often takes processors more time to do math than it does for them to do a simple equations like compare x with y. this is why I think more programmers on the pc should try doing some work on embedded processors to understand why things slow down with certain code and practices to avoid.


A really good project for a programmer to attempt is decoding of IR remote signals. The signals are transmitted with very specific timing sequences and if your code is off by a few microseconds you can miss entire pulses. It relies quite a bit on if statements for determining what part of the pulse train you are receiving and you have to be careful how you nest them .

An example program is here:
http://www.sbprojects.com/projects/ircontrol/nec.php

 

[iCyborg]

I don't disagree with your comments as it is a bad illustration. Regarding # 4 ... it is quite subjective.

I did not take any time to make it better according how to I like to code...but let's tweak it a bit.

bool blockingIO = ThingThatMightDoBlockingIO();  
int value = 0;  

if (blockingIO) 
{ // Comment here       
     value = 1;  
}
else if (x == 0)
{
     value = 2;
}
else
{
     value = 3;
}

return value;

You're still having the same performance problem. You want something like:

int value = 0;  

// Comment here :)
if (x == 0) 
{ 
     value = 1;  
}
else 
{ 
   bool blockingIO = ThingThatMightDoBlockingIO();  
   if (blockingIO) 
   {	
      value = 2;
   }
   else
   {
      value = 3;
   }
}

return value;

You want to test x==0 before you execute ThingThatMightDoBlockingIO(), so that you can avoid blocking IO if you don't have to. In your code, you never avoid it. And now that boolean seems redundant and introduces more nesting.

Multiple return paths are PITA if you need to release dynamic memory before returning to caller, otherwise if you're just returning error/status codes or something, it's usually more readable, at least to me:

// first check this to avoid blocking IO
if (x == 0) 
{ 
     return 1;  
}
if (ThingThatMightDoBlockingIO()) 
{	
     return 2;
}

return 3;

 

[Pia]

Multiple return paths are PITA if you need to release dynamic memory before returning to caller

Except in RAII capable languages like C++ where you don't need to do any extra work to release things on multiple return paths.

 

[TecHNooB]

I just code to get the job done... I find template classes, multiple inheritance, and parent/children object relationships much more disconcerting. O, and COM.

 

[Maximilian]

Right well as i wont be coding anything mission critical ill just focus on making it work before making it work well

Although ill take note of the calling a function (this is the same thing as a method right?) multiple times being potentially resource heavy. Calling it once and storing the value for comparison does seem better.

 

[wayliff]

You're still having the same performance problem. You want something like:

int value = 0;  

if (x == 0) 
{ 
     value = 1;  
}
else 
{ 
   bool blockingIO = ThingThatMightDoBlockingIO();  
   if (blockingIO) 
   {    
      value = 2;
   }
   else
   {
      value = 3;
   }
}

return value;

You want to test x==0 before you execute ThingThatMightDoBlockingIO(), so that you can avoid blocking IO if you don't have to. In your code, you never avoid it.

Valid point and I like your suggestion. Thanks - :thumbsup: