views:

904

answers:

11

I have inherited a project that has some huge switch statement blocks, some with 20 cases, what is a good way to rewrite these?

+5  A: 

Polymorphism. But it may not be a trivial refactoring.

Some examples and refs:

Refactoring (Googe books)

Switch Statement code smell and Polymorphism

Refactoring switch-statements

Mitch Wheat
+2  A: 

You could always use a lookup table.

Andrew Hare
+15  A: 

Why would you want to rewrite them in a different structure? If you really have 20 cases that have to be handled individually, a switch/case is the way to go. A big chain of if/then logic would be horrible to maintain.

Polymorphism is another option if you are using an object-oriented language. Each subclass would implement it's own functionality in the method.

Dana Robinson
So much for polymrorphism, eh?
Mitch Wheat
Yes, the problem is that the there is a point in code where 20 different things can happen, not that there is a switch statement. I switch statement is the best construct for this ugly situation.
jrcs3
If the code is written in C or some other non-OO language then polymorphism isn't really an option.
Dana Robinson
And now I see that this question has C# and Javascript tags. In C# I'd consider polymorphism. In Javascript OO is such an abomination that I'd just keep the switch/case :P
Dana Robinson
+1 for polymorphism
CMS
I agree, I have one project that's just one massive `switch` statement - works fine for me.
Dmitri Nesteruk
+2  A: 

There's nothing wrong with having 20 cases in a switch statement. You can tidy the code by refactoring and, at the very least, move the case processing into methods/functions.

scronide
A: 

It depends what the switch statement is doing.

If it's matching characters or strings, say in a parser, and you don't have the same set of patterns repeated everywhere in the code, then a switch statement might be ok.

If it's matching (say) an integer against a list of allowed values, you can create a base class and a set of derived classes for each value. Then, whatever generates the integer data in the first place can create an instance of the derived class with all of the switch statement "answers" instead.

A third option is to create a data structure that maps patterns to actions (i.e., functions or objects with virtual methods). You can look up the switch value in this data strucutre, and execute the appropriate action.

Doug
+2  A: 

Depending on what the switch statement is evaluating, you may want to refactor it using the Strategy Pattern. Take a look at this post for an example of replacing a switch on enum values with separate classes to handle each function.

It also may be that using the switch with 20 cases may actually be the best course of action for it. As long as it's readable and each result clearly conveys what the action is there's no need to really refactor it.

Agent_9191
A: 

if it's working without major bugs (by not major I mean they don't make you pull your hair out) why bother refactor it? Don't refactor everything.

If you want, you can change it to polymorphism, but this will be a shotgun surgery, you'll probably have to refactor a whole lot more than just this switch block.

hasen j
+4  A: 

As others have pointed out, it depends on the switch statement. However, in the past I have refactored switch statements by proceeding in the following way. Suppose we have a switch statement like this, with a lot of repeating code

switch(x){
   case 1:
     makeitso("foo");
     globalLog += "foo";
   case 2:
     makeitso("bar");
     globalLog += "bar";
   case 3:
     makeitso("baz");
     globalLog += "baz";
   ...
   default:
      throw("input error");
}

the first thing to do is to recognize the parts that are common (in the real world, this will probably be a little more substantial)

makeitso([some string]);
globalLog += [some string];

and turn that into a function

function transformInput(somestring) {
     makeitso(somestring);
     globalLog += somestring;
}

then for the parts that change in each case, use a hash or an array;

var transformvalues = ["foo", "bar", "baz"];

from here we can do this:

var tvals = ["foo", "bar", "baz" ... ];
function transformInput(somestring) {
     makeitso(somestring);
     globalLog += somestring;
}
var tval = tvals[x];
if(tval!==undefined) {
     transformInput(tval);
} else {
    throw ("invalid input");
}

And with tvals factored out of the switch statement, it could even be provided externally to expand the number of cases you can handle. Or you could build it dynamically. In the real world, the switch statement will often have special cases, however. I leave that as an excercise for the reader.

Breton
A: 

In general, I think you should refactor only when you need to, such as when you want to add more features, but the current design isn't up for the task. You should then refactor without adding the new functionality, and only then add the new feature.

In other circumstances, don't bother with refactoring. Do it when you need to, otherwise there are probably more important things to do.

If you really really need to, then the Visitor Design Pattern is a common switch-case replacement, though you should note it does have drawbacks. (i.e., check out www.objectmentor.com/resources/articles/acv.pdf)

Gilad Naor
+3  A: 

Three suggestions (echoing some already given):

  1. Maybe a switch isn't as bad as you think. It can be ugly if the case blocks are large. Shorten them by extracting the logic into methods.

  2. In an OO language, polymorphism might be the answer, as many have pointed out.

  3. In a functional language, like Javascript, write a function that returns the function you need to run for whatever input. That might use a switch statement itself, or it might use a lookup table.

slim
A: 

Here's a great blog post on this that's just been posted:

http://elegantcode.com/2009/01/10/refactoring-a-switch-statement/

IainMH