views:

80

answers:

4

If I need to check multiple conditions, which is the preferred way with respect to performance

if( CND1 && CND2 && CND3 && CND4)
{
}
else
{
}

or

  if(CND1)
{
   if(CND2)
   {
      if(CND3)
      {
         if(CND4)
         {
         }
         else
         {
         }
      }
      else
      {
      }
   }  
else
   {
   }
}
    }
+4  A: 

The performance difference is negligible since it should stop checking the arguments once it has found a false one, so definitely go with the first one. It's one line compared to like, 10. It also means that your else will be much easier to handle.

nickf
Why would there be *any* performance difference?
Jon Skeet
@Johannes: In a language which doesn't have short-circuiting, the two aren't equivalent to start with - any side-effects from evaluating (say) CND2 would be observed in every case for the first version, but not for the second. I'm reasonably comfortable assuming short-circuiting here.
Jon Skeet
@Johannes, was the question not originally tagged c# then?
Mark Booth
@Jon Skeet, see my answer as to why it is possible there might be a difference in performance between the two with some languages. Though not quite in the way @nickf thought. *8')
Mark Booth
@Mark: As I said, I'm assuming it's a language which *does* have short circuiting, otherwise they're not equivalent to start with.
Jon Skeet
@Mark: No, it only had "coding-style" as tag back then.
Joey
+3  A: 

They will be identical in terms of performance, but the first form is much more readable IMO.

The only situation in which I'd split the conditions up would be if there were sets of logical conditions:

if (x && y) // x and y are related
{
   if (a && b) // a and b are related
   {
   }
}

Although an alternative there which reads nicely (but does evaluate more conditions, admittedly) is:

bool shouldPayBackDebt = x && y;
bool haveEnoughFunds = a && b;
if (shouldPayBackDebt && haveEnoughFunds)
{
    // ...
}
Jon Skeet
+1 I like the second approach you mention. I'd say always go for the most readable when performance benefits are virtually negligible.
Dan Diplo
+1  A: 

Performancewise they're the same. && is short-circuited so if CND1 is false, the rest won't be evaluated.

KennyTM
A: 

C# uses short-circuit evaluation, so performance wise, both should be identical.

In a language which does not have short-circuit evaluation, the latter would probably be more efficient, but as with all optimisation, you should benchmark to see if there is a performance gain, and decide if that gain is worth making the code significantly more complex and less maintainable.

Also, it is possible that an 'optimisation' like this might not actually be an optimisation. If the conditions are cheap, and the extra complexity increases the size of the code so that it is now (for instance) no longer fits in the cache, it may actually perform less well!

From a coding style point of view, this should really be driven by what you want to do with the else's. If you want to do one thing if all are true and another thing when any is false, pick the former. If you want to be able to do different things depending on which conditions are false, user the latter.

It's not really a matter of style, it's a matter of what you want to achieve. Certainly though, if you want to do the same thing if any are false, you should be using the first form, so you don't end up with the same repeated code in each one of the else blocks. If in doubt, given two ways to implement the same requirement, it is always a good idea to chose the simplest, clearest, most maintainable option.

Oh, just in case you don't know, if you want to eagerly evaluate conditions in C#, you can use & and | instead of && and ||. So, if you want all conditions to run (perhaps for the side effects), you can do:

if (CND1 & CND2 & CND3 & CND4) { // Eagerly evaluate all conditions.
    ...
}
else {
    ...
}

One thing to bear in mind here though, is that this does look very similar to the short-circuited version, so it is well worth commenting it, especially if C or C++ people are likely to be looking at the code.

Also, I find the following lazy construct quite useful:

ok = ok && CND;

But the strict equivalent looks far too similar:

ok = ok & CND;

As such, I make sure that code like this is re-factored to:

ok &= CND;

Which makes the difference in behaviour much more obvious.

Mark Booth