Refactoring Day 24 : Remove Arrowhead Antipattern


Today’s refactoring is based on the c2 wiki entry and can be found here. Los Techies own Chris Missal also did a very informative post on the antipattern that you can find here.

Simply put, the arrowhead antipattern is when you have nested conditionals so deep that they form an arrowhead of code. I see this very often in different code bases and it makes for high cyclomatic complexity in code.

A good example of the arrowhead antipattern can be found in this sample here:

   1: public class Security

   2: {

   3:     public ISecurityChecker SecurityChecker { get; set; }

   4:  

   5:     public Security(ISecurityChecker securityChecker)

   6:     {

   7:         SecurityChecker = securityChecker;

   8:     }

   9:  

  10:     public bool HasAccess(User user, Permission permission, IEnumerable<Permission> exemptions)

  11:     {

  12:         bool hasPermission = false;

  13:  

  14:         if (user != null)

  15:         {

  16:             if (permission != null)

  17:             {

  18:                 if (exemptions.Count() == 0)

  19:                 {

  20:                     if (SecurityChecker.CheckPermission(user, permission) || exemptions.Contains(permission))

  21:                     {

  22:                         hasPermission = true;

  23:                     }

  24:                 }

  25:             }

  26:         }

  27:  

  28:         return hasPermission;

  29:     }

  30: }

</div> </div>

Refactoring away from the arrowhead antipattern is as simple as swapping the conditionals to leave the method as soon as possible. Refactoring in this manner often starts to look like Design By Contract checks to evaluate conditions before performing the work of the method. Here is what this same method might look like after refactoring.

   1: public class Security

   2: {

   3:     public ISecurityChecker SecurityChecker { get; set; }

   4:  

   5:     public Security(ISecurityChecker securityChecker)

   6:     {

   7:         SecurityChecker = securityChecker;

   8:     }

   9:  

  10:     public bool HasAccess(User user, Permission permission, IEnumerable<Permission> exemptions)

  11:     {

  12:         if (user == null || permission == null)

  13:             return false;

  14:  

  15:         if (exemptions.Contains(permission))

  16:             return true;

  17:  

  18:         return SecurityChecker.CheckPermission(user, permission);

  19:     }

  20: }

</div> </div>

As you can see, this method is much more readable and maintainable going forward. It’s not as difficult to see all the different paths you can take through this method.

This is part of the 31 Days of Refactoring series. For a full list of Refactorings please see the original introductory post.

Refactoring Day 23 : Introduce Parameter Object