views:

147

answers:

8

Which of the following is a better way to structure nesting of If statments.

if (x && y)
   doXY();
else if (x)
   doX();
else if (y)
   doY();

(OR)

if(x)
   if(y)
     doXY();
   else
     doX();       
else if(Y)
   doY();
+4  A: 

The first has less nesting so id say that.

In the second one you are evaluating wether "x" is true, then entering into that block and evaluating if "y" is true, its generally good practice to nest code as little as possible.

if (x && y)
{
   // doXY
}
else if (x)
{
   // doX
}
else
{
   // doY
}
kyndigs
the else should be //Do exceptional things. There should be a third else if that captures (y)
Woot4Moo
True I was just being lazy!
kyndigs
+1  A: 

I think the first one is better for readability as well as well structuredness

Sachin Shanbhag
A: 

I like to think of lines of code as a maintenance cost. Fewer lines = lower cost. In your case the first code block is one line shorter, so I vote for that.

Generally this would of course depend on the context of the statement.

Check out this Wiki page on SLOC

Crassy
+2  A: 

Readability-wise I would definitely go with the first option.

But if that's part of some logic that executes millions of times, and you know the distribution of probabilities for both x & y being true vs. only x being true, or only y being true, favors one of the latter, you may want to sacrifice readability for the sake of performance. However, make sure you profile before jumping to optimizations like that, and make sure you document the reasoning for structuring the if statements like that so other developers won't just come in and do you a favor by refactoring your code.

Miky Dinescu
+2  A: 

Another possibility:

if (x && y)
   doXY();
if (x && !y)
   doX();
if (!x && y)
   doY();

This is - I state up front - less efficient, but to such a minuscule extent that it would hardly ever matter. From the readability and maintainability standpoint, it may be better in some circumstances, because none of the clauses depends on any of the others and there is no requirement that they be executed in the order specified. Each individual clause could be extracted into its own method, if desired.

Carl Manaster
redundant checks in the second and third if statements. If it is rewritten as: if(x) doX() AND if(y) doY() . It nets the same result without the need of having a redundant check
Woot4Moo
@Woot4Moo: I hate using logical operators as "if" statements, in general. I think lots of people share that feeling.
David Thornley
A: 

Are the actions in DoXY mutually exclusive from the Actions DoX() and DoY()? Ie: can you re-work the actions to work like this:

if (x) { DoX(); }
if (y) { DoY(); }
if (X && y) { DoXY(); }

Of maybe even as Paramters to DoXY()

if (X || y) { DoXY(x,y); }

But I would probably go with the first one of your options for readability...

My Other Me
+1  A: 

You can also do the following:

switch (x + (y<<1))
{
   case 1: doX();  break;
   case 2: doY();  break;
   case 3: doXY(); break;
}

Disclaimer: Note that this is neither faster nor better readable, just an alternative, which might in rare cases be a acceptable solution.

codymanix
It's a sweet hack.
Ravi Gummadi
Top marks for cleverness, bottom marks for readability
Don
A: 

1.) As kyndigs said, less nesting is a good practice.

2.) Another good advice is to surround the block of operators with brackets { } no matter if there is only one called method or more.

3.) Always try to simplify your "if" statement. I mean if (isTrue) is better than if (!(!isNotFalse))

I would write the code above in this way:

if (x && y)
{
   doXY();
}
else if (x)
{
   doX();
}
else if (y)
{
   doY();
}
flyingbear