Nested Code

Nested Code

CoderFirst Team CoderFirst Team 3 months ago
293
  • #Rules for code
  • #Nested
  • #Refactoring

I don’t believe in hard rules for code. You hear them often. Things like methods shouldn’t be more than 15 lines. Methods should only have one return statement. Indent 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. Even when properly formatted, nested code is harder 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: 

if ($condition) {
  $accumulation += $item;
} else { }

That's right, an empty else block. I've also seen the opposite - an empty if 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 over 20 years. Empty blocks are dead code, remove them.

Condensed conditionals 

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;
  }
}

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 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;
}

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.

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;
    }
  }
}

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;
}

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

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;
}

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. The only way to combat this complexity is readability. However, we're unlikely to ever win the battle. Loops will always challenge us

I don't want to 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 code base contains lots of such loops, see if higher order functions like filter/map/reduce can be used. While this likely won't improve readability, it will improve your skillset. 

Closing example 

Let's revisit the code from the closing example in Formatting and apply these practices to remove the nested code. 

function check($scp, $uid) {
  if (Auth:: user() -> hasRole('admin')) {
    return true;
  } else {
    switch ($scp) {
      case 'public':
        return true;
        break;
      case 'private':
        if (Auth:: user() -> id === $uid) {
          return true;
        }
        break;
      default:
        return false;
    }
    return false;
  }
}

First, we'll remove the else as the initial conditional is already acting as a guard clause with the return true.

function check($scp, $uid) {
  if (Auth:: user() -> hasRole('admin')) {
    return true;
  }
  switch ($scp) {
    case 'public':
      return true;
      break;
    case 'private':
      if (Auth:: user() -> id === $uid) {
        return true;
      }
      break;
    default:
      return false;
  }
}

This decreases the nested code slightly and brings the switch statement back to the top level. Now that it's in focus, we see it's perceived complexity. It's verbose with just two cases. One case contains nested code. The default case hides the primary action. In addition, this block of code now mixes if and switch statements which lacks Symmetry.

Any one of these reasons might be enough to convert the switch to if statements. All of them combined makes the decision easy

function check($scp, $uid) {
  if (Auth:: user() -> hasRole('admin')) {
    return true;
  }
  if ($scp === 'public') {
    return true;
  }
  if ($scp === 'private' && Auth:: user() -> id === $uid) {
    return true;
  }
  return false;
}

In the end, we are left with a series of guard clauses. All are at the top level making the conditions easy to read through. If none are satisfied, we reach the primary action of returning false.

Do you think this post may be interesting for anyone else? Help someone gain knowledge by sharing it!