How to refactor this huge switch statement?

Question

I've inherited some code and within it is a 500 line switch statement. Basically, it switches on a string task and executes the corresponding actions.

I have since moved each case statement in to their own method in a new class. The giant switch statement still exists but instead of inlining the logic each case just calls a method so it's much neater.

The problem is that the methods modify a lot of different things. 50% of the methods require 0 arguments passed in. Some 40% require 5 arguments and the remaining 10% require 10 arguments each.

Currently this works but I'd like to make it better. Either get rid of the switch statement or lower the amount of passed in parameters somehow.

I was thinking of using a Dictionary that mapped strings to Actions to eliminate the entire switch, but this wouldn't work because I'm using a lot of ref parameters (primitive types) and there'd be no way to pass those in to the constructor and have them later be referentially modified.

The obvious solution to that problem is just to place all 16 or so variables in to a separate class and pass that but a lot of them aren't very related so it just replaces one problem with another (long parameter list with non-cohesive data class).

Was wondering if there were any other ways to improve this code. Thanks for reading.

@jcolebrand: The application is performance sensitive and involves moving machinery. This part of the program is a bottleneck so I'd like to avoid reflection. Not sure how an interface would help. Perhaps you could elaborate? — John Smith, Oct 17 '12 at 03:11
It's a little difficult to respond without any kind of context about what the switch statement does and how it relates to the rest of the code. Are the conditions and actions of the switch all related, or is this just kind of a catch-all type of scenario that should be completely removed? — pvanhouten, Oct 17 '12 at 03:17
Agree with @pvanhouten. A snippet of code showing the switch statement and the first few cases might help give us a feel for what you have. — hatchet - done with SOverflow, Oct 17 '12 at 03:21
What do you mean by "there'd be no way to pass those in to the constructor and have them later be referentially modified". You can define a delegate with `ref` parameters and then just modify all your methods to use this delegate's signature. — Mike Zboray, Oct 17 '12 at 03:21
@pvanhouten: I was thinking about adding code context but it wouldn't help. Each case statement currently calls a method. The method can take anywhere between 0 and 10 parameters. The parameters are bools, ints, objects, you name it. Some need to be modified and hence use the ref keyword. — John Smith, Oct 17 '12 at 03:24
possible duplicate of [Large Switch statements: Bad OOP?](http://stackoverflow.com/questions/505454/large-switch-statements-bad-oop) — Luke Girvin, Jan 23 '14 at 11:35

score 3 · Answer 1 · answered Oct 17 '12 at 03:32

Without being able to look at any kind of code, the only advice I can give is that you is that you should think about refactoring with testing in mind using SOLID design principles. I would try to create different classes for each module of logic (or condition of the switch), pass in dependencies through the constructors of those objects (rather than as parameters of methods) and try to create some uniform interfaces that you can use to work in some tests. You may want to extract the conditional creation of those objects by throwing in a factory. Sounds like a mess though. Good luck.

score 3 · Accepted Answer · answered Oct 17 '12 at 03:46

Since your question includes no code, the answer can't really either. I think the best thing to do is to point you to page 82 of one of the all-time best software books: Refactoring: Improving the Design of Existing Code.

"One of the most obvious symptoms of object-oriented code is its comparative lack of switch statements. Most times you see a switch statement you should consider polymorphism."

He then lists some of the specific patterns to be used to help make this happen.

score 1 · Answer 3 · answered Oct 17 '12 at 03:36

You can use ref parameters in delegates but you can't use the built-in Action or Func generic delegates. You must define your own like so:

public delegate void DelegateWithRefParameters(ref int i, ref long l, ref bool b, ref object o);

public class Program
{
    public static void Main(string[] args)
    {
        int i = 0;
        long l = 0;
        bool b = false;
        object o = null;

        var lookup = new Dictionary<string, DelegateWithRefParameters>() 
        {
            { "object", ModifyObject },
            { "int", ModifyInt },
            { "bool", ModifyBool },
        };

        string s = "object";

        lookup[s](ref i, ref l, ref b, ref o);
    }

    private static void ModifyObject(ref int i, ref long l, ref bool b, ref object o)
    {
        o = new object();
    }

    private static void ModifyInt(ref int i, ref long l, ref bool b, ref object o)
    {
        i++;
    }

    private static void ModifyBool(ref int i, ref long l, ref bool b, ref object o)
    {
        b = !b;
    }              

}

You will just have to modify all your methods to use the same signature.

How to refactor this huge switch statement?

3 Answers3

Linked

Related