views:

359

answers:

3

Continuing on previous questions (here, and here), I implemented a basic command pattern, created my command classes and coded to an interface, so when any command is used, to call the execute() method.

However, I am still finding myself unable to shake those case statements: I am reading each character from a master/decision String, which is made up of random, repeating characters A, B, C or D, and then I retrieve the related implementation of the command from a map and call its execute method.

My design was like this:

public interface Command {
    void execute();
}

public class CommandA implements Command{
  //implements execute() method
}

private Map myMap= new HashMap();
myMap.put("A", new CommandA);
myMap.put("B", new CommandB);
myMap.put("C", new CommandC);
myMap.put("D", new CommandD);

But, then when I read each instruction, again I have to resort to case statements:

switch(instructionFromString){
case 'A':{myMap.get("A").execute(); break;}
case 'B':{myMap.get("B").execute(); break;}
case 'C':{myMap.get("C").execute(); break;}
case 'D':{myMap.get("D").execute(); break;}

Obviously, somewhere along the way I managed to defeat the advantage of polymorphism against the case statements.

Could it be the kind of data structure I selected to store my commands? It can very well be a permanent data structure to just pull those commands from.

Another thing that comes to mind is the key/value naming that I used in my map. The way I tried to conceptually link each stored command to its associated instruction? i.e. Implementation of command "A", is stored on the map with key 'A' so it can match to the corresponding instruction "A"? That seems to me a bit unlikely though.

Any hint or further advice about my next move to remove those case statements once and for all would be highly appreciated. Many thanks in advance

+11  A: 

I may be missing something here, but instead of the switch statement, what's wrong with

((Command)myMap.get(instructionFromString)).execute();

If instructionFromString is a char, the convert it into a String before doing the map lookup, else use Character keys in your map.

Also, if you use a Java 5 generic map, then you can remove the cast to Command. A cleaned up version would be:

private Map<Character, Command> myMap = new HashMap<Character, Command>();
myMap.put('A', new CommandA());
myMap.put('B', new CommandB());
myMap.put('C', new CommandC());
myMap.put('D', new CommandD());

followed by:

char instructionFromString = ....
myMap.get(instructionFromString).execute();
skaffman
@skaffman: Sorry. I managed somehow to read the code sample but missed the text above and below it.
Dirk
+1 for answer I "cite", and expand on.
KLE
@skaffman: Thanks, quite elegant asnwer. Can't believe I missed that!
denchr
Also, use generics. Don't use raw collections
A: 

With a simple map, things can get nasty since you're using the same instance of CommandA all over again.

A good way to encapsulate this kind of behavior would be a factory:

public class CommandFactory
{
    public Command CreateCommand(String instruction)
    {
        if (instruction.equals("A"))
            return new CommandA();
        else if ...
    }
}

Another way would be the prototype pattern (aka clone pattern) which allows for a more flexible handling of multiple differents types (commands for that matter):

public class CommandFactory
{
    private Map<String, Command> commands = new HashMap<String, Command>();

    public void RegisterCommand(String instruction, Command commandTemplate)
    {
        commands.put(instruction, commandTemplate);
    }

    public Command CreateCommand(String instruction)
    {
        return commands.get(instruction).clone();
    }
}

As you might have noticed, you'll have to implement clone behavior into Commands, which may be time consuming.

Bryan Menard
A: 

I like Skaffman's answer. I only want to add some more:

For naming your commands, you could use a simpler pattern. For example, you could use the command's name, as a default case.

  • In the general case, that so much less to type. It eliminates errors.
  • When the name is different, you still configure the map for this specific case.

Many technologies are available to relate the name and command. Examples:

  • Annotations for the class, that specifies the command name (with a default that is equals to the name of the class).
  • Spring configuration is such a big Map already, and directly delivers the corresponding object, with many more services available along the way.
  • Java enums are also naturally associating a name with an object. Moreover, they let you have completion and compile-time checking in your IDE, along with "reference search" ans other goodies.
KLE
@KLE: Thanks! Quite useful hints there too.
denchr