Refactoring Day 18 : Replace exception with conditional

Today’s refactoring doesn’t come from any place specifically, just something I’ve picked up over time that I find myself using often. Any variations/comments would be appreciated to this approach. I think there’s some other good refactorings around these type of problems.

A common code smell that I come across from time to time is using exceptions to control program flow. You may see something to this effect:

   1: public class Microwave
   2: {
   3:     private IMicrowaveMotor Motor { get; set;}
   5:     public bool Start(object food)
   6:     {
   7:         bool foodCooked = false;
   8:         try
   9:         {
  10:             Motor.Cook(food);
  11:             foodCooked = true;
  12:         }
  13:         catch(InUseException)
  14:         {
  15:             foodcooked = false;
  16:         }
  18:         return foodCooked;
  19:     }
  20: }

Exceptions should only be there to do exactly what they are for, handle exceptional behavior. Most of the time you can replace this type of code with a proper conditional and handle it properly. This is called design by contract in the after example because we are ensuring a specific state of the Motor class before performing the necessary work instead of letting an exception handle it.

   1: public class Microwave
   2: {
   3:     private IMicrowaveMotor Motor { get; set; }
   5:     public bool Start(object food)
   6:     {
   7:         if (Motor.IsInUse)
   8:             return false;
  10:         Motor.Cook(food);
  12:         return true;
  13:     }
  14: }

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

About Sean Chambers

I am a Senior software developer from Palm Coast, Florida. An advocate of Domain Driven Design, Behavior Driven Development, creator of FluentMigrator and community activist. I am married to my beautiful wife Erin and am the proud father of two wonderful children. I currently reside at ACI, a local insurance industry/mortgage software company that excels in creating solutions using Agile methodologies.
This entry was posted in Uncategorized. Bookmark the permalink. Follow any comments here with the RSS feed for this post.
  • I think the general idea is sound, but it’s worth noting that this kind of code is vulnerable to race conditions, so the exception may still need to be thrown by Motor.Cook(food)

  • Eyston

    Maybe it is just a bad example … but having to check a boolean result seems very c’ish doesn’t it?

    If I call microwave.Start(food) and the microwave can’t do it… is an exception appropriate? It seems like a message back to the caller on why it can’t start would be useful … but I don’t know if an exception is an abuse or not (ie it doesn’t seem that exceptional).

  • Going with the example, it may depend on why the microwave can’t start. It may be in use, it may be broken, it may not be plugged in. Returning false isn’t going to shed much light on the problem. Sometimes just knowing something failed isn’t enough.

    Both examples however do just that (return false). Perhaps it is better to combine conditional checking (eg, IsInUse, IsPluggedIn, IsBroken, etc.) with throwing the appropriate exception, enabling calling code to take proper action, such as microwave.PlugIn() or while(microwave.InUse) { Thread.sleep(1000); }.