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();
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();
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
}
I think the first one is better for readability as well as well structuredness
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.
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.
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...
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.
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();
}