Long list of if statements in Java

Question

Hi,

Sorry I can't find a question answering this, I'm almost certain someone else has raised it before.

My problem is that I'm writing some system libraries to run embedded devices. I have commands which can be sent to these devices over radio broadcasts. This can only be done by text. inside the system libraries I have a thread which handles the commands which looks like this

if (value.equals("A")) { doCommandA() }
else if (value.equals("B")) { doCommandB() } 
else if etc. 

The problem is that there are a lot of commands to it will quickly spiral to something out of control. Horrible to look out, painful to debug and mind boggling to understand in a few months time.

Answer 1

Well there is a command pattern but it may be overkill for what you're doing. Remember KISS.

Answer 2

using Command pattern:

public interface Command {
     void exec();
}

public class CommandA() implements Command {

     void exec() {
          // ... 
     }
}

// etc etc

then build a Map<String,Command> object and populate it with Command instances:

commandMap.put("A", new CommandA());
commandMap.put("B", new CommandB());

then you can replace your if/else if chain with:

commandMap.get(value).exec();

EDIT

you can also add special commands such as UnknownCommand or NullCommand, but you need a CommandMap that handles these corner cases in order to minimize client's checks.

Answer 3

If it does a lot of things, then there will be a lot of code, you cant really get away from that. Just make it easy to follow, give the variables very meaningful names, comments can help too...

Answer 4

Well I suggest to create command objects and put them into a hashmap using the String as Key.

Answer 5

Just use a HashMap, as described here:

• http://www.equivalence.co.uk/archives/99

Answer 6

Have an enum of commands:

public enum Commands { A, B, C; }
...

Command command = Commands.valueOf(value);

switch (command) {
    case A: doCommandA(); break;
    case B: doCommandB(); break;
    case C: doCommandC(); break;
}

If you have more than a few commands, look into using the Command pattern, as answered elsewhere (although you can retain the enum and embed the call to the implementing class within the enum, instead of using a HashMap). Please see Andreas or jens' answer to this question for an example.

Answer 7

if it was possible to have an array of procedures(what you called commands) that'd be useful..

but you could write a program to write your code. It's all very systematic if(value='A') commandA(); else if(........................ e.t.c.

Answer 8

Even if I believe the command pattern approach is more in toward best pratices and maintainable in the long run, here's a one liner option for you:

org.apache.commons.beanutils.MethodUtils.invokeMethod(this,"doCommand"+value,null);

Answer 9

if you have multiple imbricated 'if' statements, then this is a pattern for using a rule engine. See, for example JBOSS Drools.

Answer 10

i usually try to solve it that way:

public enum Command {

A {void exec() {
  doCommandA();
}},

B {void exec() {
 doCommandB();
}};

abstract void exec();
 }

this has many advantages:

1) it is not possible to add an enum without implementing exec. so you won't miss an A.

2) you will not even have to add it to any command map, so no boilerplate code for building the map. just the abstract method and its implementations. (which is arguably also boilerplate, but it won't get any shorter..)

3) you will save any wasted cpu cycles by going through a long list of if's or calculating hashCodes and doing lookups.

edit: if you don't have enums but strings as source, just use Command.valueOf(mystr).exec() to call the exec method. note that you must use the public modifier on execif you want to call it from another package.

Answer 11

My suggestion would be a kind of lightweight combination of enum and Command object. This is an idiom recommended by Joshua Bloch in Item 30 of Effective Java.

public enum Command{
  A{public void doCommand(){
      // Implementation for A
    }
  },
  B{public void doCommand(){
      // Implementation for B
    }
  },
  C{public void doCommand(){
      // Implementation for C
    }
  };
  public abstract void doCommand();
}

Of course you could pass parameters to doCommand or have return types.

This solution might be not really suitable if the implementations of doCommand does not really "fit" to the enum type, which is - as usual when you have to make a tradeoff - a bit fuzzy.

Answer 12

You could use java reflection techniques to call the method based upon the text command ie (mangled java but you may get the idea):

String command = "doCommand" + value;

Method method = commandObject.getClass().getMethod( command,null );
// call it

Object whatever = method.invoke();

Answer 13

You're probably best off using a Map of Commands.

But is you have a set of these to handle you end up with loads of Maps knocking about. Then it is worth looking at doing it with Enums.

You can do it with an Enum without using switches (you probably don't need the getters in the example), if you add a method to the Enum to resolve for "value". Then you can just do:

Update: added static map to avoid iteration on each call. Shamelessly pinched from this answer.

Commands.getCommand(value).exec();

public interface Command {
    void exec();
}

public enum Commands {
    A("foo", new Command(){public void exec(){
        System.out.println(A.getValue());
    }}),
    B("bar", new Command(){public void exec(){
        System.out.println(B.getValue());
    }}),
    C("barry", new Command(){public void exec(){
        System.out.println(C.getValue());
    }});

    private String value;
    private Command command;
    private static Map<String, Commands> commandsMap;

    static {
        commandsMap = new HashMap<String, Commands>();
        for (Commands c : Commands.values()) {
            commandsMap.put(c.getValue(), c);    
        }
    }

    Commands(String value, Command command) {
        this.value= value;
        this.command = command;
    }

    public String getValue() {
        return value;
    }

    public Command getCommand() {
        return command;
    }

    public static Command getCommand(String value) {
        if(!commandsMap.containsKey(value)) {
            throw new RuntimeException("value not found:" + value);
        }
        return commandsMap.get(value).getCommand();
    }
}

Answer 14

I'm not sure if you have any overlap between the behaviour of your various commands, but you might also want to take a look at the Chain Of Responsibility pattern which could provide more flexibility by allowing multiple commands to handle some input values.

Answer 15

Implementing an interface as demonstrated simply and plainly by dfa is clean and elegant (and "officially" supported way). This what the interface concept is meant for.

In C#, we could use delegates for programmers who like to use functon pointers in c, but DFA's technique is the way to use.

You could have an array too

Command[] commands =
{
  new CommandA(), new CommandB(), new CommandC(), ...
}

Then you could execute a command by index

commands[7].exec();

Plagiarising from DFA's, but having an abstract base class instead of an interface. Notice the cmdKey which would be used later. By experience, I realise that frequently an equipment commmand has subcommands too.

abstract public class Command()
{
  abstract public byte exec(String subCmd);
  public String cmdKey;
  public String subCmd;
}

Construct your commands thus,

public class CommandA
extends Command
{
  public CommandA(String subCmd)
  {
    this.cmdKey = "A";
    this.subCmd = subCmd;
  }

  public byte exec()
  {
    sendWhatever(...);
    byte status = receiveWhatever(...);
    return status;
  }
}

You could then extend generic HashMap or HashTable by providing a key-value pair sucking function:

public class CommandHash<String, Command>
extends HashMap<String, Command>
(
  public CommandHash<String, Command>(Command[] commands)
  {
    this.commandSucker(Command[] commands);
  }
  public commandSucker(Command[] commands)
  {
    for(Command cmd : commands)
    {
      this.put(cmd.cmdKey, cmd);
    }
  }
}

Then construct your command store:

CommandHash commands =
  new CommandHash(
  {
    new CommandA("asdf"),
    new CommandA("qwerty"),
    new CommandB(null),
    new CommandC("hello dolly"),
    ...
  });

Now you could send controls objectively

commands.get("A").exec();
commands.get(condition).exec();