I recently came across some code that was causing an exception because a value was added to an enumeration, and the code never got updated to handle the new value. Fixing it is pretty simple. Just add another case statement, and be done with it.

But fixing it correctly is a bit more work. And any time I'm editing code, my goal is to leave it better than it was when I started. While adding the handling for the missing enumeration value seems fine, it's really not, because it's just putting a band-aid on the issue.

So what's the correct way to fix this? Well, like most things, "correct" is subjective, and as our understanding of software development changes, so does the definition of correct, so take my solution as it is: my current favorite way to handle situations like this.

First, let's look at the offending code. This is a form that shows a keyboard in our application, and it can be configured differently depending on what is being entered.

   1: private void ShowKeyboard()

   2: {

   3:   KeyboardRequest request = null;

   4:   

   5:   switch (entryMode) 

   6:   {

   7:     case EntryModes.SerialNumber:

   8:       request = GetKeyboardRequestForSerialNumber(View.EnteredValue);

   9:       break;

  10:  

  11:     case EntryModes.BarCode:

  12:       request = GetKeyboardRequestForSerialNumber(View.EnteredValue);

  13:       break;

  14:   

  15:     case EntryModes.LotNumber:

  16:       request = GetKeyboardRequestForLotNumber(View.EnteredValue);

  17:       break;

  18:   

  19:     case EntryModes.ValveNumber:

  20:       request = GetKeyboardRequestForValveNumber(View.EnteredValue);

  21:       break;

  22:   }

  23:   

  24:   var response = GetKeyboardEntryFor(request);

  25:  

  26:   if (response.Status == ServiceResultStatus.Success)

  27:   {

  28:     View.EnteredValue = response.Data;

  29:   }

  30: }

This code has a few problems. First, the main functionality is hidden because the majority of the method is a giant switch statement. Yeah, that section could be extracted out. But that won't solve the second issue, which is that this code has to be updated if we ever add another entry mode for this class.

Moving this to use a dictionary solves both issues. First, let's look at how the code changes

   1: private IDictionary<EntryModes, Func<string, KeyboardRequest>> getKeyboardRequest = 

   2:   new Dictionary<EntryModes, Func<string, KeyboardRequest>>

   3:   {

   4:     { EntryModes.SerialNumber, value => GetKeyboardRequestForSerialNumber(value) },

   5:     { EntryModes.BarCode, value => GetKeyboardRequestForBarCode(value) },

   6:     { EntryModes.LotNumber, value => GetKeyboardRequestForLotNumber(value) },

   7:     { EntryModes.ValveNumber, value => GetKeyboardRequestForValveNumber(value) },

   8:   };

   9:  

  10: private void ShowKeyboard()

  11: {

  12:   var request = getKeyboardRequest[entryMode](View.EnteredValue);

  13:   var response = GetKeyboardEntryFor(request);

  14:  

  15:   if (response.Status == ServiceResultStatus.Success)

  16:   {

  17:     View.EnteredValue = response.Data;

  18:   }

  19: }

The method is now much easier to see figure out what it is doing because the focus of the method isn't on how the request is created.

With the way it's currently written, it doesn't solve the second issue - this code needing to be modified when a new enumeration value needs to be added, but it's a lot easier to extract the initialization of the dictionary to be external to the class (passed in via the constructor and/or built by an IoC container). But, even with the way it's done now, you don't to modify the method to add a new enumeration value.

I've used this method quite a bit to clean up my code, and so far, I've been happy with the results. It makes the code a lot cleaner, and easier to maintain.

Discuss this post