One of the compound constructs that exists in virtually all programming languages is the if-elseif-elseif-...-else block.  Nearly every language in use today (except for the most esoteric) has this kind of statement.  Some have a fancier, souped up version like switch in C/C++/C#/Java, match in OCAML or case in Haskell, but the basic idea is the same: a chain of conditional statements, each dependent on all of the previous conditionals.  It’s one of the first things that any programmer learns.

However, using the else-if construct can introduce subtle, hard to find bugs, and it’s easy to see why.  Imagine this code:

if (A)
{
     // 2 pages of code
}
else if (B)
{
     // Another 2 pages of code
}
else
{
     // Do something
}

Now, obviously, any if statement with blocks that big is a candidate for refactoring, but bear with me for a moment.   The code above can be rewritten as follows, while maintaining the same semantics (assuming there are no side-effects in A or B, or anything in the if body which modifies anything that A or B refer to (thanks, Phil)):

if (A)
{
     // 2 pages of code
}

if (!A && B)
{
     // Another 2 pages of code
}

if (!A && !B)
{
     // Do something
}

The reason this construct can introduce bugs is that the conditions under which the else-if block will be executed depends upon the conditions in the if condition above, which probably isn’t even visible in your editor when you’re looking at the else-if condition.  To figure out when the else-if block will be executed, you need to take the boolean inverse of A (which can be a little confusing if it’s a compound boolean statement and you need to apply DeMorgan’s laws) and AND it with the B conditional. And to find when the else clause will be executed, you need to take the inverse of both A and B and AND them together. Each additional else-if clause makes subsequent else clause conditions harder to derive.

I know what you’re thinking, “I learned this stuff in first year computer science, it’s easy.”  Fine, but one of the best things you can do to improve the quality of your software is to manage complexity in your code.  Else-if is something that we have to use, obviously, but I’d like to make the following humble suggestions to use it more responsibly:

1. Don’t use more than 3 total clauses in a chain. That means if-elseif-else is the limit.  If your logic is more complicated than this, I suggest adding a function in the middle to dispatch to the various sub-cases, except in very simple situations.  It’s also a good opportunity to add some “self-documenting” code if you give your dispatch methods good names.
2. Refactor the conditional expressions into separate methods. Any conditional expression with more than 2 sub-expressions should probably be refactored into a separate method with a name that indicates what case the compound check is trying to handle.  Hey, it’s “self-documenting”, too!
3. Refactor the bodies of each case into separate methods. That way, it’s easy to see all of the different conditions on one screen of text.
4. Use switch instead. Or whatever compound conditional statement your language supports, keeping in mind that switch and the like generally only support looking at equality between a variable and another variable or literal, rather than any arbitrary boolean expression.
5. Always have an else or default case. Even if it doesn’t do anything, it’s a great place for a comment about when/why all of the tests will fail, or you can put Debug.Assert(false); there if it should never be reached, to catch cases (during testing, not after release) when your callers are potentially passing bad values.

Some C and C++ programmers may be preoccupied with maximizing the efficiency of the compiler-generated assembly code for multiple conditional branches, and may write their code accordingly.  These people are verifiably insane, and their ideas should be dismissed with extreme prejudice.

August 5th, 2008 | Tags: debugging, refactoring | Subscribe to comments | Leave a comment | Trackback URL