views:

3240

answers:

12

I've always been of the opinion that large switch statements are a symptom of bad OOP design. In the past, i've read articles that discuss this topic and they have provided altnerative OOP based approaches, typically based on polymorphism to instantiate the right object to handle the case.

I'm now in a situation that has a monsterous switch statement based on a stream of data from a TCP socket in which the protocol consists of basically newline terminated command, followed by lines of data, followed by a end marker. The command can be one of 100 different commands, so i'd like to find a way to reduce this monster switch statement to something more management.

I've done some googling to find the solutions I recall, but sadly, google has become a wasteland of irrelevant results for many kinds of queries these days.

Is anyone aware of any patterns for this sort of problem? Any suggestions on possible implementations?

One thought I had was to use a dictionary lookup, matching the command text to the object type to instantiate. This has the nice advantage of merely creating a new object and inserting a new command/type in the table for any new commands.

However, this also has the problem of type explosion. I now need 100 new class, plus I have to find a way to interface them to the data model cleanly. Is the "one true switch statement" really the way to go?

I'd appreciate your thoughts, opinions, or comments. Thanks.

A: 

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.

Otávio Décio
+2  A: 

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.

Jason Punyon
+21  A: 

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.

nezroy
Agree with dictionary and delegate idea. You will probably also get a performance improvement using Dictionary<string,Action> over a big switch statement. The difficulty is in initializing it, though this can be automated using reflection if desired.
Zooba
Perhaps the initialization could be pulled from configuration.
davogones
The problem with reflection is that many web hosts runs in a trust-level that don't allow you to use it :(
Interestingly enough if the only difference between the branches is that they are different subclasses of a base then you are basically describing a factory.
Thedric Walker
A: 

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

Loren Pechtel
+13  A: 

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.

ChrisW
The problem is that you don't know if you receive a This or a That command over the network. Somewhere you have to decide whether to call new This() or new That(). After that abstracting with OO is a piece of cake.
A: 

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.

ididak
I'm curious what compiler implements switch statements of strings using perfect hashing?
ceretullis
+5  A: 

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.

Jonas Kölker
A: 

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.

cyclo
The problem is that this data is coming from elsewhere, it can't be object oriented as it comes in. Something needs to be done to convert it to object orientation and generally that's a large switch.
Loren Pechtel
A: 

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.

Rine
A: 

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 ;)

devio
A: 

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).

quamrana
+1  A: 

you can use a dictionary (or hash map if you are coding in java) (it's called Table driven development by steve mcconell).

Nicolas Dorier