Large Switch statements: Bad OOP?

One way I see you could improve that would make your code driven by the data, so for example for each code you match something that handles it (function, object). You could also use reflection to map strings representing the objects/functions and resolve them at run time, but you may want to make some experiments to assess performance.

I see the strategy pattern. If I have 100 different strategies...so be it. The giant switch statement is ugly. Are all the Commands valid classnames? If so, just use the command names as class names and create the strategy object with Activator.CreateInstance.

You may get some benefit out of a Command Pattern.

For OOP, you may be able to collapse several similar commands each into a single class, if the behavior variations are small enough, to avoid a complete class explosion (yeah, I can hear the OOP gurus shrieking about that already). However, if the system is already OOP, and each of the 100+ commands is truly unique, then just make them unique classes and take advantage of inheritance to consolidate the common stuff.

If the system is not OOP, then I wouldn't add OOP just for this... you can easily use the Command Pattern with a simple dictionary lookup and function pointers, or even dynamically generated function calls based on the command name, depending on the language. Then you can just group logically associated functions into libraries that represent a collection of similar commands to achieve manageable separation. I don't know if there's a good term for this kind of implementation... I always think of it as a "dispatcher" style, based on the MVC-approach to handling URLs.

I think this is one of the few cases where large switches are the best answer unless some other solution presents itself.

I see having two switch statements as a symptom of non-OO design, where the switch-on-enum-type might be replaced with multiple types which provide different implementations of an abstract interface; for example, the following ...

switch (eFoo)
{
case Foo.This:
eatThis();
break;
case Foo.That:
eatThat();
break;
}


switch (eFoo)
{
case Foo.This:
drinkThis();
break;
case Foo.That:
drinkThat();
break;
}

... should perhaps be rewritten to as ...

IAbstract
{
void eat();
void drink();
}


class This : IAbstract
{
void eat() { ... }
void drink() { ... }
}


class That : IAbstract
{
void eat() { ... }
void drink() { ... }
}

However, one switch statement isn't imo such a strong indicator that the switch statement ought to be replaced with something else.

The best way to handle this particular problem: serialization and protocols cleanly is to use an IDL and generate the marshaling code with switch statements. Because whatever patterns (prototype factory, command pattern etc.) you try to use otherwise, you'll need to initialize a mapping between a command id/string and class/function pointer, somehow and it 'll runs slower than switch statements, since compiler can use perfect hash lookup for switch statements.

The command can be one of 100 different commands

If you need to do one out of 100 different things, you can't avoid having a 100-way branch. You can encode it in control flow (switch, if-elseif^100) or in data (a 100-element map from string to command/factory/strategy). But it will be there.

You can try to isolate the outcome of the 100-way branch from things that don't need to know that outcome. Maybe just 100 different methods is fine; there's no need to invent objects you don't need if that makes the code unwieldy.

Yes, I think large case statements are a symptom that one's code can be improved... usually by implementing a more object oriented approach. For example, if I find myself evaluating the type of classes in a switch statement, that almost always mean I could probably use Generics to eliminate the switch statement.

You could also take a language approach here and define the commands with associated data in a grammar. You can then use a generator tool to parse the language. I have used Irony for that purpose. Alternatively you can use the Interpreter pattern.

In my opinion the goal is not to build the purest OO model, but to create a flexible, extensible, maintainable and powerful system.

I'd say that the problem is not the big switch statement, but rather the proliferation of code contained in it, and abuse of wrongly scoped variables.

I experienced this in one project myself, when more and more code went into the switch until it became unmaintainable. My solution was to define on parameter class which contained the context for the commands (name, parameters, whatever, collected before the switch), create a method for each case statement, and call that method with the parameter object from the case.

Of course, a fully OOP command dispatcher (based on magic such as reflection or mechanisms like Java Activation) is more beautiful, but sometimes you just want to fix things and get work done ;)

There are two things that come to mind when talking about a large switch statement:

1. It violates OCP - you could be continuously maintaining a big function.
2. You could have bad performance: O(n).

On the other hand a map implementation can conform to OCP and could perform with potentially O(1).

You can use a dictionary (or hash map if you are coding in Java) (it's called table driven development by Steve McConnell).

I have recently a similar problem with a huge switch statement and I got rid off the ugly switch by the most simple solution a Lookup table and a function or method returning the value you expect. the command pattern is nice solution but having 100 classes is not nice I think. so I had something like:

switch(id)
case 1: DoSomething(url_1) break;
case 2: DoSomething(url_2) break;
..
..
case 100 DoSomething(url_100) break;

and I've changed for :

string url =  GetUrl(id);
DoSomthing(url);

the GetUrl can go to DB and return the url you are looking for, or could be a dictionary in memory holding the 100 urls. I hope this could help anyone out there when replacing a huge monstrous switch statements.

Think of how Windows was originally written in the application message pump. It sucked. Applications would run slower with the more menu options you added. As the command searched for ended further and further towards the bottom of the switch statement, there was an increasingly longer wait for response. It's not acceptable to have long switch statements, period. I made an AIX daemon as a POS command handler that could handle 256 unique commands without even knowing what was in the request stream received over TCP/IP. The very first character of the stream was an index into a function array. Any index not used was set to a default message handler; log and say goodbye.