Using switch and enumerations as substitute for named methods

Question

This pattern pops up a lot. It looks like a very verbose way to move what would otherwise be separate named methods into a single method and then distinguished by a parameter.

Is there any good reason to have this pattern over just having two methods Method1() and Method2() ? The real kicker is that this pattern tends to be invoked only with constants at runtime-- i.e. the arguments are all known before compiling is done.

public enum Commands
{
    Method1,
    Method2
}

public void ClientCode()
{
    //Always invoked with constants! Never user input.
    RunCommands(Commands.Method1);
    RunCommands(Commands.Method2);
}

public void RunCommands(Commands currentCommand)
{
    switch (currentCommand)
    {
        case Commands.Method1:
            // Stuff happens
            break;
        case Commands.Method2:
            // Other stuff happens
            break;
        default:
            throw new ArgumentOutOfRangeException("currentCommand");
    }
}

Answer 1

You could argue that this pattern allows you to put shared logging (or other) code for method entry and exit in a single place. But I wouldn't. AOP is a better approach for this sort of thing.

Answer 2

I can't see any obvious advantages. Quite the opposite; by splitting the blocks into separate methods, each method will be smaller, easier to read and easier to test.

If needed, you could still have the same "entry point" method, where each case would just branch out and call another method. Whether that would be a good or bad idea is impossible to say without knowing more about specific cases. Either way, I would definitely avoid implementing the code for each case in the RunCommands method.

Answer 3

If RunCommands is only ever invoked with the names constants, then I don't see any advantage in this pattern at all.

The only advantage I see (and it could be a big one) would be that the decision between Method1 and Method2 and the code that actually executes the choice could be entirely unrelated. Of course that advantage is lost, when only constants are ever used to invoke RunCommand.

Answer 4

if the code being run inside each case block is completely separate, no value added. however, if there is any common code to be executed before or after the parameter-specific code, this allows it to not be repeated.

still not really the best pattern, though. each separate method could just have calls to helper methods to handle the common code. and if there needs to be another call, but this one doesn't need the common code in front or at the end, the whole model is broken (or you surround that code with and IF). at this point, all value is lost.

so, really, the answer is no.

Answer 5

To an OO programmer, this looks horrible.

The switch and enum would need synchronised maintenance and the default case seems like make-work.

The OO programmer would substitute an object with named methods: Then the names like method1 would only appear once in the library. Also all the default cases would be obviated.

Yes, your clients still need to be synchronised with the methods you supply - a static language always insists on method names being known at compile time.

Answer 6

That pattern could be valid if you needed the coupling to be very loose. For example you might have an interface

interface CommandProcessor{
  void process(Command c);
}

If you have a method per command then each time you add a new command you would need to add a new method, if you have multiple implementations then you would need to add the method to each Processor. This could be resolved by having some base class, but if the needs diverge you could end up with a very deep class heirarchy as you add new abstraction layers (or you may already be extending another class in with the processor. If it is based on switch's over the constant you can have you default case that handles new cases appropriately by default (exceptions, whatever may be appropriate).

I have used a pattern similar to this in my code with the addition of a factory. The operations started as a small set, but I knew they would be increasing, so I had a mechanism to describe the command and then a factory that produced CommandProcessors. The factory would generate the appropriate processor and then the single method of that processor would accept the command and perform its processing.

That said if your list of command is fairly static and you don't need to worry about how tightly things are coupled then the one-method-per-command approach certainly lends itself to much more readable code.