Switch statements are bad?

Question

I recently learned that switch statements are bad in OOP, perticularly from "Clean Code"(p37-39) by Robert Martin.

But consider this scene: I'm writing a game server, receiving messages from clients, which contain an integer that indicates player's action, such as move, attack, pick item... etc, there will be more than 30 different actions. When I'm writing code to handle these messages, no metter what solutions I think about, it will have to use switch somewhere. What pattern should I use if not switch statement?

See [Large Switch statements: Bad OOP? ](http://stackoverflow.com/questions/505454/large-switch-statements-bad-oop) — Ryan Li, Dec 11 '10 at 14:01
Always find this interesting since apparently changing code is adding another case in the switch statement but adding another class isn't...sometimes the OO purist can get a bit "religious"... — Aaron McIver, Dec 11 '10 at 14:04
@Aaron, often adding a class minimises the amount of existing code that has to change. If you have code that depends on the specific classes that already exist, those are probably switch statements (or multibranch if/then/else), and each of these will need changing — The Archetypal Paul, Dec 11 '10 at 14:09
@Aaron, when adding a `case` to your switch, you need to do regression tests and develop more test cases for that same class. Whereas adding another class doesn't need that many tests to be developed as it is a "new functionality" and you're not touching any existing source, depending on how you load the new classes (aka reflection), otherwise regression tests would have to be done also for the *instantiating* code — Yanick Rochon, Dec 11 '10 at 14:31
@Paul @Yanick I understand the OO stance, I do. However there have times when developing a small app and the need for a single switch statement served fine on an enum. There was no value in going the OO route; unless I was attempting to pacify OO purist. — Aaron McIver, Dec 12 '10 at 03:31
@Aaron - sure, if the app is small or it's not likely to change much then a switch may be clearer. Sometimes "goto"s are OK too. I was just pointing out there are good reasons why people say adding a class is qualitatively ifferent from adding a switch — The Archetypal Paul, Dec 12 '10 at 09:05
"which contain an integer that indicates player's action" - To start with, this is not OO. In the OO realm, such a design is said to be plagued by "primitve obsession". "What pattern should I use if not switch statement?" - The _Strategy Pattern_ or the _Command Pattern_ most probably fits your scenario. — Powerslave, Sep 29 '16 at 08:11

score 41 · Accepted Answer · answered Dec 11 '10 at 14:04

41

A switch is like any other control structure. There are places where it's the best/cleanest solution, and many more places where it's completely inappropriate. It's just abused way more than other control structures.

In OO design, it's generally considered preferable in a situation like yours to use different message types/classes that inherit from a common message class, then use overloaded methods to "automatically" differentiate between the different types.

In a case like yours, you could use an enumeration that maps to your action codes, then attach an attribute to each enumerated value that will let you use generics or type-building to build different Action sub-class objects so that the overloading method will work.

But that's a real pain.

Evaluate whether there's a design option such as the enumeration that is feasible in your solution. If not, just use the switch.

answered Dec 11 '10 at 14:04

Toby

7,354
3
25
26

3

I guess you meant dynamic polymorphism rather than static polymorphism (*overloaded methods*) – Geek Mar 17 '13 at 16:26
@Toby Can you please elaborate this statement with example :"then attach an attribute to each enumerated value that will let you use generics or type-building to build different Action sub-class objects so that the overloading method will work."? – beinghuman Sep 14 '17 at 06:00
Yeah, I'd probably just make a `PlayerAction` enum with a `public static PlayerAction forActionId(int actionId)` and the enum would have a public `doAction` type method. No need for a switch, just `PlayerAction.forActionId(number).doAction(args);`. Now all you have to do to add another action is add an enum value and implement the `doAction` method on it. NOTE: I don't like using "ordinal" either, as it's easy to re-order your enum values alphabetically and break anything using the old numbers. – Shadow Man Mar 29 '18 at 23:40

score 22 · Answer 2 · answered Dec 11 '10 at 14:05

'Bad' switch statements are often those switching on object type (or something that could be an object type in another design). In other words hardcoding something that might be better handled by polymorphism. Other kinds of switch statements might well be OK

You will need a switch statement, but only one. When you receive the message, call a Factory object to return an object of the appropriate Message subclass (Move, Attack, etc), then call a message->doit() method to do the work.

That means if you add more message types, only the factory object has to change.

What about doing things like `Map, Thing>` this is very similar to "doing a switch on classes", is this however, taken as an okay practice? — YoTengoUnLCD, Jun 20 '16 at 20:14

score 20 · Answer 3 · answered Dec 11 '10 at 13:59

The Strategy pattern comes to mind.

The strategy pattern is intended to provide a means to define a family of algorithms, encapsulate each one as an object, and make them interchangeable. The strategy pattern lets the algorithms vary independently from clients that use them.

In this case, the "family of algorithms" are your different actions.

As for switch statements - in "Clean Code", Robert Martin says that he tries to limit himself to one switch statement per type. Not eliminate them altogether.

The reason is that switch statements do not adhere to OCP.

score 5 · Answer 4 · answered Dec 11 '10 at 14:01

5

I'd put the messages in an array and then match the item to the solution key to display the message.

answered Dec 11 '10 at 14:01

Michael Swierzbinski II

51
1

not everything has to be OOP. Really like this answer. – grasshopper Feb 07 '14 at 09:57

score 5 · Answer 5 · answered Dec 11 '10 at 16:11

I don't buy it. These OOP zealots seem to have machines that have infinite RAM and amazing performance. Obviously with inifinite RAM you don't have to worry about RAM fragmentation and the performance impacts that has when you continuously create and destroy small helper classes. To paraphrase a quote for the 'Beautiful Code' book - "Every problem in Computer Science can be solved with another level of abstraction"

Use a switch if you need it. Compilers are pretty good at generating code for them.

You're supposed to refactor then optimize. Doing it the other way around doesn't make any sense. — Eva, Jan 27 '13 at 20:30

score 4 · Answer 6 · answered Dec 11 '10 at 14:06

From the design patterns perspective you can use the Command Pattern for your given scenario. (See http://en.wikipedia.org/wiki/Command_pattern).

If you find yourself repeatedly using switch statements in the OOP paradigm, this is an indication that your classes may not be well design. Suppose you have a proper design of super and sub classes and a fair amount of Polymorphism. The logic behind switch statements should be handled by the sub classes.

For more information on how you are removed these switch statements and introduce the proper sub-classes, I recommend you read the first chapter of Refactoring by Martin Fowler. Or you can find similiar slides here http://www1.informatik.uni-wuerzburg.de/database/courses/pi2_ss03_dir/RefactoringExampleSlides.pdf. (Slide 44)

Yanick Rochon · Answer 7 · 2010-12-11T14:24:43.223

IMO switch statements are not bad, but should be avoided if possible. One solution would be to use a Map where the keys are the commands, and the values Command objects with an execute() method. Or a List if your commands are numeric and have no gaps.

However, usually, you would use switch statements when implementing design patterns; one example would be to use a Chain of responsibility pattern to handle the commands given any command "id" or "value". (The Strategy pattern was also mentionned.) However, in your case, you might also look into the Command pattern.

Basically, in OOP, you'll try to use other solutions than relying on switch blocks, which use a procedural programming paradigm. However, when and how to use either is somewhat your decision. I personally often use switch blocks when using the Factory pattern etc.

A definition of code organisation is :

a package is a group of classes with coherant API (ex: Collection API in many frameworks)
a class is a set of coherent functionalities (ex: a Math class...
a method is a functionality; it should do one thing and one thing only. (ex: adding an item in a list may require to enlarge that said list, in which case the add method will rely on other methods to do that and will not perform that operation itself, because it's not it's contract.)

Therefore, if your switch statement perform different kinds of operations, you are "violating" that definition; whereas using a design pattern does not as each operation is defined in it's own class (it's own set of functionalities).

score 2 · Answer 8 · answered Dec 11 '10 at 14:45

Use commands. Wrap the action in an object and let polymorphism do the switch for you. In C++ (shared_ptr is simply a pointer, or a reference in Java terms. It allows for dynamic dispatch):

void GameServer::perform_action(shared_ptr<Action> op) {
    op->execute();
}

Clients pick an action to perform, and once they do they send that action itself to the server so the server doesn't need to do any parsing:

void BlueClient::play() {
    shared_ptr<Action> a;
    if( should_move() ) a = new Move(this, NORTHWEST);
    else if( should_attack() ) a = new Attack(this, EAST);
    else a = Wait(this);
    server.perform_action(a);
}

Switch statements are bad?

8 Answers8

Linked

Related