DEV Community

Cover image for Untangling Nested Code
Jason McCreary
Jason McCreary

Posted on

Untangling Nested Code

I don’t believe in hard rules for writing code. You hear them often. Things like methods shouldn’t be more than 15 lines. Methods should only have one return statement. Indentation must be 4 spaces. Such rules are too rigid.

In the real world code is much more fluid. Adhering to these hard rules distracts us from what really matters - readability. If my focus is strictly on the number of lines or return statements, I prevent myself from writing more readable code simply because it is a few lines "too long" or has more than one return statement.

Many of these hard rules attempt to address nested code. Nested code is hard to follow. Physically there’s more visual scanning with the eyes. Mentally, each level of nesting requires more overhead to track functionality. All of these exhaust a reader.

Nested code is mostly the result of conditionals. Since conditionals are the basis for all programming logic, we can’t very well remove them. We must recognize their effect on readers and take steps to minimize this impact.

Getting back on top

To improve readability, we want to bring the code back to the top level. By nature, loops and conditionals have a nested structure. There's no way to avoid nesting within these blocks. However, we can avoid nesting beyond this structure.

Let's take a look at a few examples of nested code and practices for improving their readability.

Empty blocks

You may not believe me, but I've seen the following code more than once:

public function handle($request, Closure $next)
{
    if (env('APP_ENV') == 'development') {
        // do nothing...
    } else {
        if ($request->getScheme() != 'https') {
            URL::forceScheme('https');
            return redirect('https://www.example.com/' . $request->getPath());
        }
    }

    return $next($request);
}
Enter fullscreen mode Exit fullscreen mode

That's right, an empty if block. I've also seen the opposite - an empty else block. There is no rule that an if must be paired with an else - at least not in any of the programming languages I've used in the past 20 years. Empty blocks are dead code, remove them.

Conditional values

Nested code blocks often return a value. When these are boolean values, there's an opportunity to condense the code block and return the condition directly.

Consider the nested code within the isEmpty method of a Set class:

public function isEmpty() {
    if ($this->size === 0) {
        return true;
    } else {
        return false;
    }
}
Enter fullscreen mode Exit fullscreen mode

While this method block is only 4 lines of code, it contains multiple sub-blocks. Even for such a small number of lines this is hard to read, making the code appear more complex than it really is.

By identifying the conditional return of raw boolean values we have the rare opportunity to completely remove the nested code by directly returning the condition.

public function isEmpty() {
    return $this->size === 0;
}
Enter fullscreen mode Exit fullscreen mode

Given the context of this aptly named method combined with a now one line block, we decreased its perceived complexity. Although this line may appear dense, it is nonetheless more readable than the original.

Note: Condensing conditionals can work for more data types than raw booleans. For example, you can return the condition as an integer with a simple type cast. However this rapidly increases the complexity. Many programmers try to combat this by using a ternary. But a ternary condenses the code without decreasing the complexity, making the code less readable. In these cases, a guard clause is a better alternative.

Guard clauses

Nested code is often the result of logical progression. As programmers we write out each condition until we reach a level where it's safe to perform the action.

While this flow may be ideal for execution, it's not ideal for reading. For each nested level the reader has to maintain a growing mental model.

Consider the following implementation of the add method of a Set class:

public function add($item) {
    if ($item !== null) {
        if (!$this->contains($item)) {
            $this->items[] = $item;
        }
    }
}
Enter fullscreen mode Exit fullscreen mode

Logically the progression: if the item is not null and if the Set does not contain the item, then add it.

The problem is not only the perceived complexity of such a simple action, but that the primary action of this code is buried at the deepest level.

Ideally the primary action of a block of code is at the top level. We can refactor the conditional to a guard clause to unravel the nested code and expose the primary action.

A guard clause simply protects our method from exceptional paths. Although they commonly appear at the top of a code block, they can appear anywhere. We can convert any nested conditional into a guard clause by applying De Morgan's Laws and relinquishing control. In code, this means we negate the conditional and introduce a return statement.

By applying this to the add method our implementation becomes:

public function add($item) {
  if ($item === null || $this->contains($item)) {
      return;
  }

  $this->items[] = $item;
}
Enter fullscreen mode Exit fullscreen mode

In doing so we have not only drawn out the primary action, but emphasized the exceptional paths for our method. It is now less complex for future readers to follow. It's also easier to test as the exceptional paths are clearly drawn out.

Switching to if

A switch statement is a very verbose code structure. switch inherently has 4 keywords and 3 levels. It's a lot to read even if the contained blocks of code are only a few lines. While this is acceptable in certain cases, it's not in others.

There are a few cases where using if statements instead of a switch statement may produce more readable code.

  • When there are just a few case blocks the inherent structure of the switch statement produces more lines than the equivalent if statements.
  • When case blocks contain nested code the complexity increases and readability decreases to a critical level. Using guard clauses or adopting the practices within Big Blocks can improve the code.
  • When type casts or calculations are needed to coax the value into the constraints of a switch. This doesn't apply to languages (Swift, Go, etc) that support more complex case comparisons.

switch statements are best when a 1:1 ratio exists between case statements and the lines of code within their blocks. Whether these lines are assignments, return statements, or method invocations the readability remains nearly the same provided the ratio is roughly 1:1.

switch ($command) {
    case 'action':
        startRecording();
        break;
    case 'cut':
        stopRecording();
        break;
    case 'lights':
        adjustLighting();
        break;
}
Enter fullscreen mode Exit fullscreen mode

Note: In such cases where switch statements are streamlined, many programmers use a map, database table, or polymorphism instead. All of which are indeed additional alternatives. Just remember every solution has trade-offs (complexity). A switch statement is often "good enough" for most code.

Complex loops

Another common form of nested code are loops. Loops by nature are complex. As programmers, we're cursed to be off by one and miss incremental logic. Again, we're humans not computers. So we're unlikely to win the battle against loops. They will always challenge us. The only way to combat this complexity is readability.

I won't get into which data structures and algorithms may help improve your code. That is much too specialized. In general though, most loops deal with accumulation or invocation. If you find your codebase contains lots of loops, see if higher order functions like filter/map/reduce can be used. While this may not improve readability for all readers, it will improve your individual skillset.


Want more tips? This practice is taken from BaseCode - a field guide containing 10 real-world practices to help you improve the code you write every day.

Top comments (9)

Collapse
 
hatem20 profile image
Hatem Said • Edited

another bad practice I keep doing in looping,
when I need to create another array form the array I am looping on
I solve this by creating a new array with the same name prefixed by an underscore. but I think there should be a cleaner solution

$_attributeValues = [];

foreach ($attributeValues as $attributeValue) {
foreach ($attributeValue as $attribute => $attributeValueCode) {
$_attributeValues[$attribute][] = $attributeValueCode;
}
}

return $_attributeValues;

Collapse
 
alainvanhout profile image
Alain Van Hout

Typically, when you're transforming an array into another array, there is a specific task that you're trying to accomplish. The name of the new array could reflect that.

Beyond that, method extraction and functional programming can be your friend in these cases.

Collapse
 
gonedark profile image
Jason McCreary

As noted, definitely take a look at the array or collection functions available. The code you posted looks like it is a native group or flatten function.

Collapse
 
erclairebaer profile image
Claire McCrea

Really valuable article, thank you!

Collapse
 
gonedark profile image
Jason McCreary

Glad you found it valuable. More to come from BaseCode. Also, where's the "ry" on the end of your name?

Collapse
 
erclairebaer profile image
Claire McCrea

I'm too lazy for it ¯_(ツ)_/¯

Collapse
 
qm3ster profile image
Mihail Malo

Amazingly well written explanations and justifications, I will be quoting this. :)

Just can't agree that cases are often good enough though.

Maybe because I don't write PHP? :/

Collapse
 
itaditya profile image
Aditya Agarwal

Instead of switch statements we can also use objects. The keys act as cases. Now we just have to call decisionObject[case]. It also frees us from break statements

Collapse
 
gonedark profile image
Jason McCreary

Completely agree for simple blocks like the examples. Just remember to use good naming.