Replace Switch/Case with Pattern

Question

I have code very similar to this example three times in a code behind. Each time the switch is toggling off of an option that is sent to it. Each time the code inside the case is exactly the same except for a parameter based of off the case. Is using a switch/case and methods the best way to do this? Should I think of using sometype of design pattern to avoid the repeative switch/case structure?

string option = dropDownList.SelectedValue.ToString();
switch (option.ToUpper())
{
 case "ALPHA":
  // do repeative code method here; only change is a parameter
  break;
 case "BRAVO":
  // do repeative code method here; only change is a parameter
  break;
 case "CHARLIE":
  // do repeative code method here; only change is a parameter
  break;
 case "DELTA":
  // do repeative code method here; only change is a parameter
  break;
 default:
  break;
}

Answer 1

Perhaps a transform from option to the method parameter?

This would allow you to drop the switch statement and simply feed the method the transformed parameter.

Answer 2

You can construct a table to convert string to parameter value.

var lookup = new Dictionary<string, ParaType> ()  
{
    { "ALPHA", a  },
    { "BETA", b },
    ....
};

ParaType para;
if (lookup.TryGetValue(option, out para))   // TryGetValue, on popular request
{       
   // do repeative code method here; with para
}

Answer 3

The key to this problem is making the objects stored in the dropDownList provide the parameter (either directly or by indexing into an array). Then the switch statement may be removed completely.

If the parameter is a property of the object that appears in the drop down list, then this will provide the value very efficiently.

If the values in the drop down list can provide a numeric index into an array of parameter values, then this will beat a series of string comparisons in terms of runtime efficiency.

Either of these options are cleaner, shorter, and easier to maintain than switching on a string.

Answer 4

This is probably overkill for what you're doing but the code could end up cleaner.

class AlphabetChar
{
    public virtual void DoSomething(){}
}

class Alpha : AlphabetChar {}
class Bravo : AlphabetChar {}
...

class AlphabetCharFactory
{
    public static AlphabetChar GetByName(string name)
    {
         switch (name.ToUpper())
         {
             case "ALPHA":
                   return new Alpha();

             ...

             default:
                  //google for "wiki null object"
                  return new NullAlphabetChar();
         }
    }
}

Then the calling code becomes...

string option = dropDownList.SelectedValue.ToString();

AlphabetChar alphabetChar = AlphabetCharFactory.GetByName(option);

alphabetChar.DoSomething();

Like I said, this is overkill if all you're doing is setting a parameter (basically setting a property in the base class from the child class). But if your logic is more complex, this could be a viable option for you.

Answer 5

I think I would move the switch statement into a separate function and have it return the parameter value for each case:

    private static string GetParameterForAllCases(string option)
    {
        switch (option.ToUpper())
        {
            case "ALPHA":
                return "ALPHA Parameter";
            case "BRAVO":
                return "BRAVO Parameter";
            case "CHARLIE":
                return "CHARLIE Parameter";
            case "DELTA":
                return "DELTA Parameter";
            default:
                return "Default Parameter";
        }
    }

Then you can just call your work method once:

        string option = dropDownList.SelectedValue.ToString();

        WorkMethod(GetParameterForAllCases(option);

---

If you don't want to execute your work method for the default cause, or if you have more than one parameter value, then you could change the GetParameter method to use output parameters:

    private static bool GetParameter(string option, out string value)
    {
        switch (option.ToUpper())
        {
            case "ALPHA":
                value = "ALPHA Parameter";
                return true;
            case "BRAVO":
                value = "BRAVO Parameter";
                return true;
            case "CHARLIE":
                value = "CHARLIE Parameter";
                return true;
            case "DELTA":
                value = "DELTA Parameter";
                return true;
            default:
                value = null;
                return false;
        }
    }

And call it like this:

        string option = dropDownList.SelectedValue.ToString();

        string value;
        if (GetParameter(option, out value))
            WorkMethod(value);

Answer 6

Compilers are very good at optimizing switch/case constructs; the CLR will likely turn it into a lookup table or something similarly fast, so hand-rolling your own version such as Henk Holterman suggests is not what I would recommend. The CLR can do a better job than you can at choosing the best algorithm.

If it's an issue of elegance or maintainability, and you have several switch/cases strewn about the same class performing similar functions, then one way to improve it is to encapsulate all of the functionality related to a single "case" into its own class instance, like so:

class MyOption
{
    public static readonly MyOption Alpha = new MyOption(1, 10, "Alpha Text");
    public static readonly MyOption Bravo = new MyOption(2, 100, "Bravo Text");
    public static readonly MyOption Charlie = new MyOption(3, 1000, "Charlie Text");
    // ... Other options ...
    public static readonly MyOption Default = new MyOption(0, 0, null);

    public MyOption(int id, int value, string text)
    {
        this.ID = id;
        this.Value = value;
        this.Text = text;
    }

    public int ID { get; private set; }
    public int Value { get; private set; }
    public string Text { get; private set; }
}

Then in your class/control/page:

static MyOption GetOption(string optionName)
{
    switch (optionName)
    {
        case "ALPHA":
            return MyOption.Alpha;
        case "BRAVO":
            return MyOption.Bravo;
        case "CHARLIE":
            return MyOption.Charlie;
        // ... Other options ...
        default:
            return MyOption.Default;
    }
}

private MyOption GetOptionFromDropDown()
{
    string optionName = GetOptionNameFromDropDown();
    return GetOption(optionName);
}

private string GetOptionNameFromDropDown()
{
    // ... Your code ...
}

After that you can start churning out events and other methods:

private void control1_SomeEvent(object sender, EventArgs e)
{
    MyOption option = GetOptionFromDropDown();
    DoSomething(option.ID);
}

private void control2_SomeEvent(object sender, EventArgs e)
{
    MyOption option = GetOptionFromDropDown();
    DoSomethingElse(option.Value);
}

Of course, this is only a useful pattern if you have several of these switch/cases that you want to refactor into one. If you've only got one switch/case, you're just going to end up with a lot more code this way, so leave it alone!

Other possibilities to improve maintainability include:

• Changing the string to an Enum type (convert optionName using Enum.Parse);
• Moving all the MyOption/GetOption stuff into its own class (if you have several classes/controls/pages that all need to operate on the same set of choices);
• Add a method delegate to the MyOption class if you actually need to call a different method for each;
• Have your DropDownList or other control store a direct reference to the MyOption instance, if possible.

That's about it. It's simple to write, it's easy to understand, it's easy to maintain, it will save you time if you have a lot of switch/case constructs, and it still allows the CLR to perform the best possible optimizations. The only cost is the small amount of memory required to hold those readonly fields.