Mo' Nesting Mo' Problems

This is a follow up to my previous article on writing readable code.

Programming languages give us the ability to nest statements inside each other to practically no end, and it's nice that they give us the flexibility to write and structure our code anyway we deem fit.

We are free to place an if statement inside of a for loop, and inside that place another if statement, then add a while loop, then a switch statement, etc. But just because you can, doesn't mean you should; with each nested statement that is added, the level of complexity in the code is increased, and the amount of effort it will take anyone to try to comprehend the code also goes up drastically.

Methods with too many nested statements and too much logic are code smells that indicate the code has reached a point where it should be refactored.

Problems

There are a number of problems that arise from nesting your code too much. Let's look at this example snippet and see what's potentially wrong with it:

function foo() {
    if () {
        // code
    }

    if () {
        // code
        if () {
            // code
        } else if () {
            // code
        } else {
            // code
            if () {
                // code
            }
            // code
        }
        // code
    }
    // code
}

Despite the lack of actual code, from a glance you can see that the function has a lot going; lots of if statements, lots of curly braces and lots of contexts for code to exist in. Any time you try to read this function you have to load all of the statements into memory and keep track of what lines of code fall under which conditions. It's a lot to take in and only slows you down.

If the method also contains more code and grows longer than the view port is tall, then you may fall into situations where some statements don't fit onto the screen and you have to constantly scroll to view the top and bottom areas of the statement. Again, just slows you down.

Furthermore, it can be tricky to know which curly brace belongs to which if statement because there's just too many braces in general; so it becomes hard to find your bearings inside of the function, and basic navigation around the code isn't so basic any more.

How about this one:

function bar($baz) {
    if ($baz == true) {
        // this
        // function
        // contains
        // lots
        // of
        // code
    }
}

Off the cuff, there's actually not a whole lot wrong with it, it only has the one if statement so it's pretty easy to understand; but me being the pedantic arsehole that I am, don't find it semantically pleasing that the entire body of the function lives inside of an if statement and is at indentation level 1 rather than 0.

The Rule

My rule for avoiding nested code hell is to keep the indentation level inside every method as low as possible. Personally I start to feel guilty after my code exceeds an indentation level of 2 and I start itching to break things down.

Solutions

Let's look at some example code snippets and how I would go about reducing the indentation level in each one.

Break Out of Methods Early

Remember this chestnut from earlier:

function bar($baz) {
    if ($baz == true) {
        // this
        // function
        // contains
        // lots
        // of
        // code
    }
}

Rather than wrapping the body of the function in an if statement to stop it from running, I would break out of the function early with a return statement.

function bar($baz) {
    if ($baz != true) {
        return;
    }

    // this
    // function
    // contains
    // lots
    // of
    // code
}

Sometimes this can make your code a bit more readable, sometimes it might not make a significant difference; your mileage will vary. But it's a tool in your toolbox that has its use cases, don't forget it.

Break Out of Loops Early

On top of breaking out of methods early, you can also break of out loops.

function foo($users) {
    foreach ($users as $user) {
        if ($user->hasPremiumAccount()) {
            $this->sendPremiumEmai($user->email);
            $this->chargeUser($user->id);
            // more crap...
        }
    }
}

Again, I don't find it semantically pleasing that the entire body of the foreach loop lives inside of an if statement. Here's my take on restructuring it:

function foo($users) {
    foreach ($users as $user) {
        if (!$user->hasPremiumAccount()) {
            continue;
        }

        $this->sendPremiumEmai($user->email);
        $this->chargeUser($user->id);
        // more crap...
    }
}

We break out of the current iteration of the loop early using the continue statement if the condition does not apply, if it does apply then the if statement will do nothing and proceed to execute the rest of the code in the body of the foreach loop.

This small refactor isn't going to move mountains with regards to you code's readability, but it's another tool in your toolbox that can help make a small difference.

Break Down Logic Into Separate Methods

If the aforementioned tips aren't enough, you can refactor your code further by breaking things out into separate methods.

public function foo($users)
{
    foreach ($users as $user) {
        if ($user->hasSignedUpToNewsletter()) {
            if ($user->hasPremiumAccount()) {
                $this->sendPremiumEmail($user->email);
            } else {
                $this->sendRegularEmail($user->email);
            }
        }
    }
}

In this example, all the code responsible for doing stuff exists at indentation level 3, that's way to high for my liking. A possible approach to restructuring this code would be to separate it out into more methods like so:

public function foo($users)
{
    foreach ($users as $user) {
        if ($user->hasSignedUpToNewsletter()) {
            $this->sendEmail($user);
        }
    }
}

public function sendEmail($user)
{
    if ($user->hasPremiumAccount()) {
        $this->sendPremiumEmail($user->email);
    } else {
        $this->sendRegularEmail($user->email);
    }
}

With the code in a new method, you can break down the amount of logic in a single method (thus simplifying the method), and reset the counter for the indentation level of the nested code. Another small win for readability.

These tips will only give you small wins with regards to keeping your code readable. If you make 5 small refactors to your code using these tips, then the readability of your code could go up by 5 arbitrary units; consider yourself 5 arbitrary units richer! Over time this will add up to a much more readable code base and you will be rich in high quality code.

Conclusion

To recap; my previous article said to keep your methods as short as possible, now I build on that and say to also keep your methods as thin as possible by minimising indentation.