My musings about .NET and what not

Chocolate Salty Bools

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!

kick it on DotNetKicks.com

shout it on DotNetShoutOut.com

vote it on WebDevVote.com

Bookmark / Share

    » Similar Posts

    1. Singletons vs. Static Classes
    2. Should We Return Null From Our Methods?
    3. Open Source or Die – The *Real* Future of Graffiti?

    » Trackbacks & Pingbacks

    1. You've been kicked (a good thing) - Trackback from DotNetKicks.com

      Chocolate Salty Bools — September 22, 2009 7:25 PM
    2. Thank you for submitting this cool story - Trackback from DotNetShoutout

      Chocolate Salty Bools — September 22, 2009 7:32 PM
    3. Pingback from Chocolate Salty Bools : LeeDumond.com | Diary Koki Bloon

      Chocolate Salty Bools : LeeDumond.com | Diary Koki Bloon — September 22, 2009 10:36 PM
    4. Pingback from Dew Drop – September 23, 2009 | Alvin Ashcraft's Morning Dew

      Dew Drop – September 23, 2009 | Alvin Ashcraft's Morning Dew — September 23, 2009 7:52 AM
    5. DotNetBurner - burning hot .net content

      Chocolate Salty Bools — September 24, 2009 5:44 PM
    Trackback link for this post:
    http://leedumond.com/trackback.ashx?id=75

    » Comments

    1. Jef Claes avatar

      I don't really mind the ServiceStatus enum.

      Isn't it possible that a status get's added in the future?

      Like OnHold or something.

      Jef Claes — September 23, 2009 4:09 AM
    2. Lee Dumond avatar

      @Jef -- Don't forget YAGNI...

      I think it's best to code for *current* requirements. Unnecessarily complicating things in order to accommodate a *possible* future use is not a good idea. Chances are, you ain't gonna need it.

      If you do find yourself needing to represent a third state at some point, then I'd refactor to an enum at that time.

      Lee Dumond — September 23, 2009 9:16 AM
    3. Jef Claes avatar

      I feel you, but I wouldn't take the time to refactor if the first snippet is widely used through the codebase and has no problems. You see what I'm trying to say? :)

      Jef Claes — September 23, 2009 12:21 PM
    4. John Bubriski avatar

      In response to YAGNI, I feel like this doesn't hinder, or complicate things. You have an enum comparison instead of a bool comparison. So what?

      John Bubriski — September 23, 2009 2:02 PM
    5. Lee Dumond avatar

      @John - Refactoring is all about *simplifying the code* -- making it easier to read, understand, and maintain.

      If you're actually trying to tell me that working with an enumeration is as simple as working with a bool value... well, I just don't what to say to that. ;)

      Lee Dumond — September 23, 2009 2:21 PM
    6. John Bubriski avatar

      Not going to publish my first comment?

      John Bubriski — September 23, 2009 3:16 PM
    7. Lee Dumond avatar

      John, I've gotten two comments from you on this post (including that last one), both of which are published.

      I haven't seen anything else from you in the approval queue. If there was a third one, my apologies. Please feel free to repost and it should publish immediately.

      Lee Dumond — September 23, 2009 3:28 PM
    8. Jeremy Gray avatar

      This may or may not be applicable to your example application, but here is a good case (and this is supported by the framework design guidelines) for a two-valued enumeration: To change a method that takes more than one bool argument into a method that takes at most one bool argument, as methods taking multiple bool arguments are verboten.

      Jeremy Gray — September 24, 2009 12:17 AM
    9. John Bubriski avatar

      I'll totally backup what Jef Claes said. If you ever need to add another status... you're screwed. You'll need to re-refactor your code back to an Enum.

      But why is it a code smell? What is the problem there. Sure, if you have something that only ever has 2 states, then you should use a bool. But I love Enums for their descriptive nature, and highly recommend them in a situation like this.

      Here are some possibilities:

      ScheduledForMaintenance - For when someone sets up a maintenance appointment.

      WaitingForService - For when it's in the shop but not being worked on yet.

      BeingServiced - Duh

      ServiceComplete - Duh

      PickedUp - For after service is complete, and the bike is out of the shop

      You could run some AWESOME analytics on your statuses (especially if you track them!). You could find out things like how long you have bikes waiting for service (maybe you need more workers), or how long people schedule in advance (offer discounts maybe).

      John Bubriski — September 24, 2009 8:38 AM
    10. Morten Lyhr avatar

      Is and Are = Hungarian notation.

      You might as well call them bInService.

      Also if you call a method with a bool parameter, it looks like this.

      DoSomething("foo-bar", true);

      and it even gets worse when there are multiple bool parameters.

      DoSomethingElse("foo-bar", true, false, true);

      It is much more reader frendly to use an 2 state enum.

      Correct("foo-bar", Status.Created, ServiceStatus.InService, Pickup.Emidiatly);

      Morten Lyhr — September 25, 2009 7:06 AM

    » Leave a Comment