Choosing the right design pattern for smooth cmd line arg processing

Question

My situation is as follows.

I have an abstract Command class which has an exec method. I have a series of concrete command classes that extend this abstract class.

I have a CommandFactory in the context class that creates and returns an appropriate command based on cmdline args which I parsed and sent to the factory. (no problems here, I'm able to parse cmdline args just fine).

However within the command factory I have this large list of if-else-if not unlike this

 public Command getCmd(String cmdType){
  if(cmdType == null){
     return null;
  }     
  if(cmdType.equalsIgnoreCase("CLEAN")){
     return new CleanCmd();

  } else if(cmdType.equalsIgnoreCase("KILL")){
     return new KillCmd();

  } else if(cmdType.equalsIgnoreCase("START")){
     return new StartCmd();
  }

  return null;
}

Note: The input args are a set of flags and args which are too complex for the scope of this question. You can just think of the equalsIgnoreCase as something more complex.

However I think this if else construct is kinda ugly. I'd like to replace it with something more elegant. Also correct me if I am wrong the current paradigm also violates the open-close principle because every time I add a new command I modify the factory ?

score 1 · Accepted Answer · answered Jul 24 '17 at 18:18

1

I suggest using two concepts if possible:

Command pattern like described in this answer, and using some Dependency Injection Framework like spring or guice.

With this two ideas, your code could be something like this:

class CommandFactory {
    // Using spring.
    // This will inject all the classes implementing Command.
    @Autowired
    List<Command> commandList;       

    // Mapping the command name to the implementation  
    Map<String, Command> commandMap;         

    // Initializing the command map
    @PostConstruct
    public void buildMap() {
        for (Command command : commandList) {
            commandMap.put(command.getType(), command);
        }
    }

    public Command getCmd(String cmdType) { 
        return commandMap.get(cmdType);
    }
}

With this configuration, the CommandFactory class respect the Open Close Principle. No modification needed to add a new command and yet accepts new commands on demand.

answered Jul 24 '17 at 18:18

rafaelim

644
6
13

Hmmm, but my project doesn't use Spring ! But this is interesting though – Srini Jul 24 '17 at 18:29
Can't you use some lightweight DI like https://github.com/google/guice from google? I don't see how you can avoid coding the CommandFactory without DependencyInjection. But if you come up with some other solution, tell me to learn! Good coding – rafaelim Jul 24 '17 at 18:39
Ok, I'll evaluate this and get back to you. – Srini Jul 24 '17 at 18:42
I evaluated this and ended up going with guice, thanks rafelim – Srini Jul 26 '17 at 20:19

score 0 · Answer 2 · answered Jul 25 '17 at 11:04

There is nothing wrong with doing the simplest thing, right up until it doesn't do what you need.

One way you could do exactly what you've already done, but slightly more neatly, is to use a String-based switch statement:

switch(cmdType) {
    case "KILL":
       return new KillCommand();
    case "CLEAN":
       return new CleanCommand();
    default:
       throw new UnknownCommandException("Unknown command: " + cmdType);
}

I notice your special handling of null. I urge you to try to avoid returning null anywhere. If you do this throughout, then much of the time you don't need to check for null either.

If your needs get more sophisticated, consider having a Map<String,Command>:

private static Map<String,Command> commandMap = new HashMap<>();
map.put("CLEAN", new CleanCommand());
map.put("KILL", new KillCommand());

... etc, and then:

public Command getCommand(String cmdString) {
    return commandMap.get(cmdString.toUpperCase());
}

Note that this returns the same instance each time -- this is often a good thing, if you can make sure that the command class itself is immutable and thread-safe.

If, you need a new instance each time, you have options:

Map<Class<? extends Command>> and use map.get(s).newInstance()
Map<Supplier<? extends Command>> and use map.get(s).get()
Map<SpecificCommandFactory> and use a method from SpecificCommandFactory (I would have called this just CommandFactory if you hadn't already defined this)

Using this patterns means you're ready for a number of other techniques if and when you need them:

Creating the map using dependency-injection using a framework (Spring, Guice, etc.)
Adding/removing commands at init time (e.g. from a config file)
Adding/removing commands at runtime

score -1 · Answer 3 · answered Jul 25 '17 at 10:38

I would suggest a Prototype Design Pattern in place of a Command Static Factory (if-then... ladder) you are using. As probable commands to be instantiated are finite in nature, prototype is the best pattern to generate your command. Prototype pattern will keep pre-populated repository of Commands and as and when client part asks for new Command it will obtain the required command prototype from repository and provide a clone as a concrete command.

Choosing the right design pattern for smooth cmd line arg processing

3 Answers3