In the course of reading various design patterns and refactoring books (like Refactoring), I noticed while flow control is sorta mentioned here and there, it’s not addressed especially well. It mostly encourages things like replacing conditional with polymorphism and replace nested conditional with guard clauses and the like, rather than finding ways to straighten out and clarify code that contains flow control and conditionals.
One of my thoughts on flow control and conditionals follows one of the rules of the Zen of Python:
Flat is better than nested.
When looking at nested code, there is mental stack involved in trying to figure out what’s going on, as well as having to correlate close braces with any code that follows the end of the prior conditional. Thus, flat code is easier to visually parse than nested code.
Another drive in refactoring of conditionals and flow control is:
The easiest code to debug is the code you don’t need to look at.
Therefore, code that can return from a function, or break from a loop sooner is better than making you stick around and read the whole thing when you really didn’t need to. Unfortunately, some languages make this a bit harder than they should (as much as I love lisps, they tend to have this problem). Either way, it’s still worthwhile.
One last principle that drives the way I refactor conditional and flow control logic is:
Code, at least within a function, is read top-down.
Depending on your display setup in your dev environment, you can’t count on being able to see the end of any given block, whether it’s a conditional block, or looping block, so making it more likely that you’ll see the full result of a conditional branch can make things just that much better.
A flow control (FC) statement, in the sense that I am using it in this
post is a return, continue, break, goto or exception throw
statement. These are independent of any language in particular.
Putting shorter clause first
If you have an if-then-else conditional, put the shorter of the then and else first. I’ve often seen code where the then clause goes on for twenty lines with a single line else clause which looks ridiculous. Negate the conditional and swap the then and else clauses.
Removing Else
If you have an if-then-else conditional where the then clause ends in FC, having an else clause is superfluous, remove the else statement, and outdent it’s body.
if (cond) {
do_something();
return;
} else {
do_something_else();
}
is equivalent to:
if (cond) {
do_something();
return;
}
do_something_else();
If your then clause does not end in FC, but your else clause does, you can invert the conditional (swapping the then and else clauses), and apply the previous transformation.
Hoisting Following FC
If your then and/or else clause is long-ish and is followed directly by FC, you can hoist the FC to the ends of both clauses. If the clause already ends in FC, don’t hoist to that clause.
if (cond) {
do_something();
} else {
do_something_else();
}
return;
Can be transformed into:
if (cond) {
do_something();
return;
} else {
do_something_else();
return;
}
Then, with the previous transformation applied:
if (cond) {
do_something();
return;
}
do_something_else();
return;
Manifesting Implicit FC
Manifesting implicit FC can be used to enable the previous transformations. Implicit FC’s are things like:
- returns at end of functions that don’t return anything
- continues at the end of a loop body
For example:
for (init; cond; step) {
if (cond) {
do_something();
} else {
do_something_else();
}
}
At the end of the loop body, there’s an implicit continue statement. Adding that in we get:
for (init; cond; step) {
if (cond) {
do_something();
} else {
do_something_else();
}
continue;
}
We can then hoist that into the conditional:
for (init; cond; step) {
if (cond) {
do_something();
continue;
} else {
do_something_else();
continue;
}
}
Then applying the else-removing transformation:
for (init; cond; step) {
if (cond) {
do_something();
continue;
}
do_something_else();
continue;
}
Then we can remove the redundant continue statement:
for (init; cond; step) {
if (cond) {
do_something();
continue;
}
do_something_else();
}
Removing Redundant If
If the then and else clauses merely return true and false, the if statement can be removed, and just return the value of the conditional. When you see this:
if (cond) {
return true;
} else {
return false;
}
Fix it to be this:
return cond;
Obviously, if it’s false/true instead of true/false, negate the condition.
Adding Else
This is good when you have a conditional where the then is long and ends in FC, but the stuff that follows it to the next FC (manifested or implied) is very short. To the end of having the shorter bit first:
void foo() {
if (cond) {
long_series_of_things();
long_series_of_things();
...
FC
}
shorter_thing();
return; // IMPLICIT_FC
}
We add an else clause containing the rest, including the implicit FC:
void foo() {
if (cond) {
long_series_of_things()
long_series_of_things();
...
FC
} else {
shorter_thing()
HOISTED_IMPLICIT_FC
}
}
Then, swap the conditional:
void foo() {
if (!cond) {
shorter_thing()
HOISTED_IMPLICIT_FC
} else {
long_series_of_things()
long_series_of_things();
...
FC
}
}
Remove else:
void foo() {
if (!cond) {
shorter_thing()
HOISTED_IMPLICIT_FC
}
long_series_of_things()
long_series_of_things();
...
FC
}
Summary
Guard clauses are a good thing if they cause the function or loop to return/continue/break early. They make it so that if the rest of the loop/function doesn’t apply to me, I don’t necessarily have to read it. With the above transformations, you should be able to improve code readability by reducing nesting, and clarifying your logic. If you add in method extraction, that can also be used to help. And also, as a guide point that isn’t hard and fast: more than three nesting levels is a code smell.
Happy refactoring!
Edit: added goto to the list of FC statements