LeeDumond.com

A couple of Boolean-related anti-patterns, and what to do about them.

---

You’ve probably noticed a lot of bloggers use their platform to vent their frustrations. This is one of those posts.

I happen to be working on a brownfield application that manages a repair shop for motorcycles. I don’t mind telling you that this particular application is a deep, dark shade of brown, and I don’t mean because it just came back from Disney World.

Modeling a business process like this often involves tracking the current state of various domain objects. The code here uses enumerations to express those states, which is a good thing. Enumerations are much more expressive than integers, far more type-safe than magic strings, and can help eliminate a lot of messy switch statements and such from your code. But one of the issues with this app is that it’s full of two-valued enumerations, like this:

namespace RepairShop
{
   public enum ServiceStatus
   {
      InService,
      ServiceComplete
   }
}

Upon first glance, this makes sense. The repair status of a motorcycle can have two states. When you take in a broken one, you mark it “InService”. When you finish fixing it, you mark it “ServiceComplete”.

public class Motorcycle
{
   public ServiceStatus ServiceStatus { get; set; }
   public string TagNumber { get; set; }

   public void TakeInForService()
   {
      this.ServiceStatus = ServiceStatus.InService;
      this.Update();
   }
   
   public void MarkServiceComplete()
   {
      this.ServiceStatus = ServiceStatus.ServiceComplete;
      this.Update();
   }

   public void Update()
   {
      MotorcycleData.Update(this);
   }      
}

Oooo, That Smell

Looking at several of these enumerations, something didn’t seem quite right. It was then I realized something – two-valued enumerations are a code smell. I’ve never seen this in any code smell list I’ve ever read, but it definitely needs to be included.

A little bit of refactoring lets us simplify things, and eliminate the enumeration type from the codebase altogether.

public class Motorcycle
{
   public bool ServiceStatus { get; set; }
   public string TagNumber { get; set; }

   public void TakeInForService()
   {
      this.ServiceStatus = true;
      this.Update();
   }
   
   public void MarkServiceComplete()
   {
      this.ServiceStatus = false;
      this.Update();
   }

   public void Update()
   {
      MotorcycleData.Update(this);
   }      
}

The simple fact is that when any property has two and only two states, that property can always be typed as a Boolean. In or Out. On or Off. Black or White. If you’ve only got two choices, why not use a perfectly good, built-in two-valued type instead of making up a new one?

He Who Smelt It, Dealt It

But hold on a minute – this refactoring isn’t quite done. When examining the value of ServiceStatus  in a Motorcycle object, one might naturally assume that ServiceStatus = true means “it’s bring worked on” and ServiceStatus = false means “it’s done.” However, this isn’t totally obvious, or even semantically meaningful. This leads us to another new code smell: Boolean Jeopardy.

Why “Boolean Jeopardy?” That’s because Boolean values are either true or false; and semantically speaking, true and false are responses to a yes-or-no question. Therefore, anything of type Boolean should always be named in the form of a question – more specifically, a binary (yes-or-no) question.

public class Motorcycle
{
   public bool IsInService { get; set; }
   public string TagNumber { get; set; }

   public void TakeInForService()
   {
      this.IsInService = true;
      this.Update();
   }
   
   public void MarkServiceComplete()
   {
      this.IsInService = false;
      this.Update();
   }

   public void Update()
   {
      MotorcycleData.Update(this);
   }      
}

You should apply this naming rule to anything that’s a Boolean: including fields, properties, bool-returning methods, even bit columns in database tables. A good way to do this is to try to start with the word ‘Is’ for booleans that apply to single entities, and ‘Are’ for collections.

internal class CookieProgram
{
   private static void Main()
   {
      IList cookies = Cookie.GetCookieBatch();

      if (AreCookiesDone(cookies))
      {
         RemoveFromOven(cookies);
      }
   }

   private static bool AreCookiesDone(IEnumerable cookies)
   {
      foreach (Cookie cookie in cookies)
      {
         if (!cookie.IsDone)
         {
            return false;
         }
      }

      return true;
   }

   private static void RemoveFromOven(IEnumerable cookies)
   {
      // blah blah blah
   }
}

Get the Funk Out

Okay, so we’ve identified a couple of Boolean-related whiffs, and come up with a couple of associated refactorings.

I call the first one Two-Valued Enumeration. Enumerations are very handy, but can be tricky to deal with in code, and can lead to unnecessary complexity when overused. Any two-valued enumeration can and should be expressed as a Boolean, which is a built-in, dual-valued primitive type. To eradicate this smell, apply the Replace Two-Valued Enumeration With Boolean refactoring.

When you do that, pay attention to the name you give your new Boolean, lest you end up with the Boolean Jeopardy smell. Since the values that a Boolean can hold are true or false, the names of Boolean entities (fields, properties, methods, database columns, and so forth) carry more semantic meaning when they appear in the form of a question. Apply the Rename Boolean As Binary Question refactoring to any offenders you find.

Happy refactoring, and thanks for letting me vent.

Subscribe to this blog for more cool content like this!

Bookmark / Share