Refactor java code

Question

Okay guess this question looks a lot like:

http://stackoverflow.com/questions/519422/what-is-the-best-way-to-replace-or-substitute-if-else-if-else-trees-in-programs

consider this question CLOSED!

---

I would like to refactor code which looks something like this:

String input; // input from client socket.
if (input.equals(x)) {
  doX();
} else if (input.equals(y)) {
  doY();
} else {
  unknown_command();
}

It is code which checks input from socket to perform some action, but I don't like the if else construction because every time a new command is added to the server (code) a new if else has to be added which is ugly. Also when deleting a command the if else has to be modified.

Answer 1

Collect those commands in a Map<String, Command> where Command is an interface with an execute() method.

Map<String, Command> commands = new HashMap<String, Command>();
// Fill it with concrete Command implementations with `x`, `y` and so on as keys.

// Then do:
Command command = commands.get(input);
if (command != null) {
    command.execute();
} else {
    // unknown command.
}

To get a step further, you could consider to fill the map dynamically by scanning for classes implementing a specific interface (Command in this case) or a specific annotation in the classpath. Google Reflections may help lot in this.

Update (from the comments) You can also consider combining the answer of Instantsoup with my answer. During the buildExecutor() method, first get the command from a Map and if the command doesn't exist in Map, then try to load the associated class and put it in the Map. Sort of lazy loading. This is more efficient than scanning the entire classpath as in my answer and creating it everytime as in Instantsoup's answer.

Answer 2

One way could be to have an interface ICommand that is the general contract for a command, e.g.:

public interface ICommand {
    /** @param context The command's execution context */
    public void execute(final Object context);
    public String getKeyword();
}

And then you could use Java's SPI mechanism to auto-discover your various implementations and register them in a Map<String,ICommand> and then do knownCommandsMap.get(input).execute(ctx) or something alike.

This practically enables you to decouple your service from command implementations, effectively making those pluggable.

Registering an implementation class with the SPI is done by adding a file named as the fully qualified name of your ICommand class (so if it's in package dummy the file is going to be META-INF/dummy.ICommand within your classpath), and then you'll load and register them as:

final ServiceLoader<ICommand> spi = ServiceLoader.load(ICommand.class);
for(final ICommand commandImpl : spi)
    knownCommandsMap.put(commandImpl.getKeyword(), commandImpl);

Answer 3

How about interfaces, a factory, and a little reflection? You will still need to handle exceptions on bad input, but you would always need to do this. With this method, you just add a new implementation of Executor for a new input.

public class ExecutorFactory
{
    public static Executor buildExecutor(String input) throws Exception
    {
        Class<Executor> forName = (Class<Executor>) Class.forName(input);
        return (Executor) executorClass.newInstance();
    }
}

public interface Executor
{
    public void execute();
}


public class InputA implements Executor
{
    public void execute()
    {
        // do A stuff
    }
}

public class InputB implements Executor
{
    public void execute()
    {
        // do B stuff
    }
}

Your code example then becomes

String input;
ExecutorFactory.buildExecutor(input).execute();

Answer 4

Building the Command patten upon an enum class can reduce some of the boilerplate code. Let's assume that x in input.equals(x) is "XX" and y in input.equals(y) is "YY"

enum Commands {
   XX {
     public void execute() { doX(); }        
   },
   YY {
     public void execute() { doY(); }        
   };

   public abstract void execute();
}

String input = ...; // Get it from somewhere

try {
  Commands.valueOf(input).execute();
}
catch(IllegalArgumentException e) {
   unknown_command();
}

Answer 5

You say that you're processing input from a socket. How much input? How complex is it? How structured is it?

Depending on the answers to those questions, you might be better off writing a grammar, and letting a parser generator (eg, ANTLR) generate the input-processing code.