Guard Conditions or Else

This is a small but significant way to clean up your code. I had a fellow programmer alert me to this technique a few years back, and it’s one of those things were now that I see the anti-pattern, I can’t un-see it.

The basic idea is, you can clean up a lot of bad nesting and fractured code by treating test conditions as guard conditions instead of if/else clauses.

As as example in Python, he’s a nasty little piece of code in a style I see way too frequently:

def ack_poo(value, things):
    """Nesting makes this really hard to read"""
    if value > 0:
        if len(things) > 0:
                    do_something()
                else:
                    raise Exception('thing must be positive')
            else:
                raise Exception('value is not in things')
        else:
            raise Exception('too many things!')
    else:
        raise Exception('value must be positive') 

At face value, it kind of makes sense. You can trace the conditions in one direction, and the failures in the other. But in practice, the conditions are more complicated than this, and the deep nesting makes matching them up really hard.

For more information, do a search on the “Arrow Anti-Pattern” or the “Mountain Anti-pattern”. I even read one article where a person had correlated bug counts against a timeline of an effort to remove deep nesting from their code, and found a strong correlation between removing nesting and reducing the number of bugs dramatically. When the code nesting fell below three, the number of new bugs dropped precipitously.

Now, the AH-HA moment comes when you realize that the conditions above result in a stopping point. If the condition fails, we don’t need to go forward. That each condition is raising an exception is a stronger hint that each “if” results in individual stopping points.

So I can rewrite the code with each of those conditions as a guard condition — if it fails, we just stop:

def flatter(value, things):
    """Nesting makes this really hard to read"""
    if value = 1000:
        raise Exception('too many things!')

    if value not in things:
        raise Exception('value is not in things')

    if things[value] <= 0:
        raise Exception('thing must be positive')

    do_something()

Now, not only is the logic flatter and much easier to read, but it’s much easier to make modifications. If I need to reorder the checks, here it’s just a single cut and paste, where above I have to cut and paste twice, then fix indentation. Adding a new check is simpler too.

Even if the code isn’t deeply nested, treating a test as a guard condition can make your code much more readable.

    if value in things:
        blah1()
        blah2()
        blah3()
        blah4()
        blah5()
        blah6()
        blah7()
        blah8()
        blah9()
        blah10()
        blah11()
        blah12()
    else:
        # I have to jump back over a big block of code to see what the test was...
        raise Exception('whaaaaaa?')

    # as a guard condition
    if not value in things:
        raise Exception('value not in things')

    blah1()
    blah2()
    blah3()
    blah4()
    blah5()
    blah6()
    blah7()
    blah8()
    blah9()
    blah10()
    blah11()
    blah12()

EDIT: WordPress mangled some of my logic because of the > and <, but that’s OK… see in which case it’s easier to match up (and fix) the logic. 😉

Advertisements

Leave a comment

Filed under design patterns, opinionizing

Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out / Change )

Twitter picture

You are commenting using your Twitter account. Log Out / Change )

Facebook photo

You are commenting using your Facebook account. Log Out / Change )

Google+ photo

You are commenting using your Google+ account. Log Out / Change )

Connecting to %s