Google Testing Blog: Code Health: Reduce Nesting, Reduce Complexity

Code Health: Reduce Nesting, Reduce Complexity

Monday, June 05, 2017

This is another post in our Code Health series. A version of this post originally appeared in Google bathrooms worldwide as a Google Testing on the Toilet episode. You can download a printer-friendly version to display in your office. Try spotting the bug

Code with too much nesting

Code with less nesting

response = server.Call(request)
 
if response.GetStatus() == RPC.OK:
  if response.GetAuthorizedUser():
    if response.GetEnc() == 'utf-8':
      if response.GetRows():
        vals = [ParseRow(r) for r in 
                response.GetRows()]
        avg = sum(vals) / len(vals)
        return avg, vals
      else:
        raise EmptyError()
    else:
      raise AuthError('unauthorized')
  else:
    raise ValueError('wrong encoding')
else:
  raise RpcError(response.GetStatus())

response = server.Call(request)
 
if response.GetStatus() != RPC.OK:
  raise RpcError(response.GetStatus())

if not response.GetAuthorizedUser():
  raise ValueError('wrong encoding')

if response.GetEnc() != 'utf-8':
  raise AuthError('unauthorized')
 
if not response.GetRows():
  raise EmptyError()

vals = [ParseRow(r) for r in 
        response.GetRows()]
avg = sum(vals) / len(vals)
return avg, vals

wrong encoding"unauthorized"This bug is easier to see in the refactored version, since the checks occur right as the errors are handled.guard clausesthe refactored version is much easier to read and maintain
Here are some rules of thumb for reducing nesting in your code:

Keep conditional blocks short. It increases readability by keeping things local.

Consider refactoring when your loops and branches are more than 2 levels deep.

Think about moving nested logic into separate functions. For example, if you need to loop through a list of objects that each contain a list (such as a protocol buffer with repeated fields), you can define a function to process each object instead of using a double nested loop.

6 comments :

UnknownJune 7, 2017 at 4:52:00 PM PDT
I would love to see some side by side examples in the style shown above where you used the rules of thumb listed. Could anyone point me in the direction of some examples? Thanks.
ReplyDelete
Replies
Radosław BondyraJune 7, 2017 at 11:00:00 PM PDT
Thanks for the article. I like the example. As a rule of thumb I'd add replace conditional with polymorphism if possible.
ReplyDelete
Replies
Bradyn PoulsenJune 21, 2017 at 9:41:00 AM PDT
Aren't the following 2 logical checks flipped on the exceptions they should raise?

if not response.GetAuthorizedUser():
•• raise ValueError('wrong encoding')

if response.GetEnc() != 'utf-8':
•• raise AuthError('unauthorized')
ReplyDelete
Replies
Bradyn PoulsenJune 21, 2017 at 9:43:00 AM PDT
Regarding my logic flip comment lol. Disregard since I clearly see it was intentional after continuing with the artcile
ReplyDelete
Replies
UnknownJuly 3, 2017 at 4:40:00 AM PDT
I recently refactored a too much nested logic check because I had the very impression it wasn't redeable. Glad to see that kind of article, I will keep this tip in mind.
ReplyDelete
Replies
Richard CattermoleAugust 15, 2017 at 8:26:00 AM PDT
The one difference I would have is if error0 else if errorN else { success }.
Slightly easier to immediately go, ah huh, error(s) then success paths. Thanks to less whitespace + usage of else keyword.
ReplyDelete
Replies