Switch, select, case, really long if statement…whatever the cause, when they get too long they all become unmanageable. Overly long switch statements do exists, I’ve written them, we have all seen them in other peoples code. In fact, I remember hearing, from a fellow Elegant Coder, that a coworker of his hit the limit for how many cases can be in a .Net switch statement. I don’t remember what the limit was, but it was a lot, and they exceeded that number.
Side Note: what do you do in that situation? Someone else wrote it, and is in a fix. In this case, the esteemed Elegant Coder smiled, said he had no idea, and walked away.
Now, what do you do if a switch statement slowly moves towards a maintenance nightmare? After all, any switch statement, with more than 10 items in it will be larger than any screen output you have — thus moving beyond the ideal of being able to see all the code in a method without scrolling.
My solution: use a Dictionary. Really, a Dictionary. The keys are hashed, retrieval by index is very fast, and we have these cool delegate things and lambda to help us along.
First though:
Chris Brandsma’s rules of proper switch statement etiquette:
- A switch statement should really be in its own method, all by itself, away from all other innocent code. I call this switch statement inoculation.
- There should be no more than one line of code in each case, not including the break;
- You should strive to use the return statement instead of relying on breaks. This means that one line of code should return something. If that is not the case…user the break.
- Best case, each case of a switch statement performs an action, and each action is in a separate class.
Following those three rules will better help you refactor switch statement code in the future. But there are still a few reasons for not liking this. First off, switch statements are very static. To add a new item to statement you have to recompile. There are times when you would like the list to be a bit more dynamic. Maybe you would like the list to be loaded via an IoC library.
Now on to the refactoing…
Step 1: Figure out what each case statement needs to run. Typically all the code in a case statement needs a common set of data to execute. Figure out what that is.
Step 2: Create a delegate for passing all of these values to each case as if it were a method.
Step 3: Convert all of the code in each case to a method that follows the delegate just created.
Step 4: Create a Dictionary that implements the delegate, and initialize the dictionary with the methods.
Step 5: Replace the switch with a single call to Invoke on the delegate.
Just show me some darn code
I’ve included a sample project, so you can just download it and check out the code from there. I’ve included samples for .Net 2.0 (C# 2.0) and .Net 3.5 (C# 3.0) in the project. I’m using .Net 3.5 code here.
For step one, here is a class that I created that contains all of the methods my switch/dictionary is going to call:
1: public class ValueProcessor : IValueProcessor
2: {
3: public string Chris(string data)
4: {
5: return "Brandsma" + data;
6: }
7:
8: public string Jason(string data)
9: {
10: return "Grundy" + data;
11: }
12:
13: public string Scott(string data)
14: {
15: return "Nickols" + data;
16: }
17:
18: public string David(string data)
19: {
20: return "Starr" + data;
21: }
22:
23: public string Tony(string data)
24: {
25: return "Rasa" + data;
26: }
27: }
Also, my original switch statement looked like this:
1: public string ReturnSomething(string name, string value)
2: {
3: switch (name)
4: {
5: case "Chris":
6: return _valueProcessor.Chris(value);
7: case "David":
8: return _valueProcessor.David(value);
9: case "Scott":
10: return _valueProcessor.Scott(value);
11: case "Jason":
12: return _valueProcessor.Jason(value);
13: case "Tony":
14: return _valueProcessor.Tony(value);
15: default:
16: return string.Empty;
17: }
18: }
Now the refactored version:
1: public Dictionary<string, Func<string, string>> CreateDictionary()
2: {
3: var dictionary = new Dictionary<string, Func<string, string>>
4: {
5: {"Chris", _valueProcessor.Chris},
6: {"David", _valueProcessor.David},
7: {"Jason", _valueProcessor.Jason},
8: {"Scott", _valueProcessor.Scott},
9: {"Tony", _valueProcessor.Tony}
10: };
11: return dictionary;
12: }
13:
14: public string ReturnSomething(string name, string value)
15: {
16: if (_dictionary.ContainsKey(name))
17: {
18: return _dictionary[name].Invoke(value);
19: }
20: return string.Empty;
21: }
When to do this
Something I’ll point out right away, there is more code in the refactored version. But removing code is not always what we are after, we are after maintainable code. For the purposes of this demo, I kept things simple, we were not dealing with a large switch statement to begin with.
If you go through the switch statement rules above and you still need something else, then bring this out. But, I do not see this as an automatic way to upgrade some code.
Next Steps
That is the thing with refactoring, you are never really done. You can move the dictionary initialization to its own class. Or you can replace the dictionary with an IoC. Depends on what you need.
Chris, Great idea on the dictionary, very clever, I am definitely going to use that in the future.
Not sure I like all the returns directly out of the case statements of the switch. In general, my preference is to have one exit point in a function if possible, although, I guess if the method only contains a switch then maybe that would be OK.
Thanks again for posting this!
Hmmm. Not sure I’m on board with this approach. Normally I refactor switch statements through polymorphism. You can still inject the desired behavior with an IoC.
I’m with Sergio there, a switch statement can usually be replaced by the careful application of polymorphism. If you refactor such that the switch statement is the only thing in a method, and each entry in the switch statement calls only another method, then the state for each case and each method can usually be grouped into its own subclass. As a bonus you’d get your cyclomatic complexity down 🙂
Turning
class X {
void m() {
switch(…) {
case A:
m_A();
break;
…
}
}
}
into
class X {
IM _m;
void m() {
_m.dostuff();
}
}
interface IM {
void dostuff();
}
class MA extends IM {
public void dostuff() {
…
}
}
This, of course can be quite verbose, so I’ve found myself using variants of the dict trick before.
Misko Hevery has some interesting insights on this in this Google Tech-talk: http://www.youtube.com/watch?v=4F72VULWFvc
@Sergio,
I would agree with you for the right problem that is the right solution — just like this. Both patterns are tools, I want as many as possible. But I’ll have to cover that one next.
It looks like your going in the direction of implementing a command pattern processor.
As stated previously, polymorphism is probably the right direction. I believe that in order to implement polymorphism you still are using a switch statement. So you’ve actually created a primitive polymorphism mechanism
See this code:
http://f1.grp.yahoofs.com/v1/wE9qSeZCY8ECA0qUbu4IT65Byp5Z9-3CwBkd9MJdo9F4P5s-muAVMEDA1Y32yv-GJCbZwQwVre5D_A-aKl1FJAhiVK_4K7c/OperationType.cs
Great idea, however it may be maintanable by the person who wrote the code, but I am not convinced a co-worker will view the code and imediatly know what is representing. They may just look confused and think ‘Why not just use a Switch statement?’ But I guess it is all dependent on the development team and the situation.
Refactor a switch statement into a dictionary remembers me my early days learning python, I asked one day “what? python doesn’t have a switch statement?” and the python users answer me with the “dictionary use”.
There still a few things to learn from languages as python 😉
I am going with Sergio on this one. A switch might be an indication of an OCP violation and I would look towards polymorphism to get rid of it entirely.
I’ve allready done this approach in a small project. Have you meassured the performance ?
I just created a switch statement with 5,000 items (I tried more but Visual Studio kept crashing). This coworker must have been an idiot with a lot of time on his hands.
I agree with JP, refactoring to patterns would point you to polymorphism.
Here’s an alternative:
http://www.netobjectives.com/PatternRepository/index.php?title=TheStrategyTemplatePattern
Problem is we have aroung 1500 switch cases in our controller is there anything we can do to refator
I’ve used this pattern too, sometimes I’ll have a domain factory class that produces little strategy objects based on some criteria. Hate using IoC in such a situation and this solution works a treat.
Hey thanks for this, I really like it I actually have some questions…
Should we EVER use a switch statement?
The only time I consider using them is when I know the code is final and that code is sitting on the very top layer almost like a script.
In my last job I was “set straight” that case statements were in fact the only way to do certain things which at the time I strongly disagreed with. For instance type checking & do such and such if its this type etc. I took it quite hard that my efforts to aim for elegance were put down and dismissed and I had to have those ideas knocked out of me! I’m still very bitter by the whole experience.
I wonder what stands in the way of these ideals, and why we have things such as case statements?