- Conditionals testing different dimensions should have different indentations
There are a number of reasons to avoid using chained "if ... else if ... else" constructs. First among these is the ease with which they hide bugs.
Chained if-else constructs can hide bugs because each conditional can test a different dimension of the state space. When this is done, successive "if" clauses explore successively smaller half-spaces in the state space. As a result, much of the matrix of possibilities in the state space is left unexplored.
What does all of that mean? We need an example.
if (A) doA();Here, I use an underscore to indicate "not", so do_AB() represents "do something when A is false and B is true".
else if (B) do_AB();
else if (C) do_A_BC();
else do_A_B_C();
If we have three predicates (A, B and C) and they are orthogonal (independent), then there are 8 possibilities in the full matrix of possibilities:
_A_B_Cyet, we have enumerated only four of these. The source code using a chained if-else construct looks neat, but it has obscured the fact that all possibilities have not been canonically examined. Time after time, I have exposed (and corrected) long-standing bugs by merely expanding a chained if-else construct to its canonical form, and filling in the missing cases.
_A_BC
_AB_C
_ABC
A_B_C
A_BC
AB_C
ABC
The simple rule-of-thumb that may be followed to achieve this is that the conditionals in a chained if-else construct must all lie along the same dimension. For example, if we have a straight enumeration:
enum signal_colorthen the following chained if-else would be permissible:
{
SIGNAL_NONE,
SIGNAL_RED,
SIGNAL_YELLOW,
SIGNAL_GREEN
};
signal_color sc = ... ;Note that every clause tests the same variable, sc, and that every test uses equality. (Bit tests are different, as shown below.) Since all the tests lie along the same dimension, it makes sense to add an else clause, to capture unhandled cases. This is good Software Engineering practice, as it guards against unexpected behavior in this code, should another element be added to the enumeration in the future.
if (sc == SIGNAL_NONE) printf("Stop, then go.\n");
else if (sc == SIGNAL_RED) printf("Stop.\n");
else if (sc == SIGNAL_YELLOW) printf("Floor it!\n");
else if (sc == SIGNAL_GREEN) printf("Go.\n");
else error("Unexpected signal_color.\n");
However, if we have a bit-mapped enumeration:
enum signal_color_componentsThen the following chained if-else is not permissible:
{
SIGNAL_COMPONENT_BLUE = 1<<0,
SIGNAL_COMPONENT_GREEN = 1<<1,
SIGNAL_COMPONENT_RED = 1<<2,
};
signal_color_components scc = ... ;In this version, it is evident that we left room for a bug, and the damned thing crawled right in! We want the case scc & SIGNAL_COMPONENT_RED && scc & SIGNAL_COMPONENT_GREEN to print out "Floor it!\n", and yet this is not done.
if (scc & SIGNAL_COMPONENT_RED) printf("Stop.\n");
else if (scc & SIGNAL_COMPONENT_GREEN) printf("Go.\n");
else printf("Stop, then go.\n");
The predicates testing each bit of a bitmap are orthogonal predicates (i.e. predicates testing different dimensions), so we fell right into the trap. If the rule of thumb is followed, we would first rewrite the above as:
signal_color_components scc = ... ;Now it is obvious that the first clause (where the signal color has a red component) is missing a case. After adding back the missing case:
if (scc & SIGNAL_COMPONENT_RED)
{
printf("Stop.\n");
}
else
{
if (scc & SIGNAL_COMPONENT_GREEN) printf("Go.\n");
else printf("Stop, then go\n");
}
signal_color_components scc = ... ;the code segment will behave as intended. Goodbye bug!
if (scc & SIGNAL_COMPONENT_RED)
{
if (scc & SIGNAL_COMPONENT_GREEN) printf("Floor it!\n");
else printf("Stop.\n");
}
else
{
if (scc & SIGNAL_COMPONENT_GREEN) printf("Go.\n");
else printf("Stop, then go\n");
}
No comments:
Post a Comment