Long list of if statements in Java

Question

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

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 2

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 3

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 4

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();

Answer 5

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

Answer 6

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 7

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 8

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 9

The answer provided by @dfa is the best solution, in my opinion.

I'm just providing some snippets in case you are using Java 8 and want to use Lambdas!

Command without parameters:

Map<String, Command> commands = new HashMap<String, Command>();
commands.put("A", () -> System.out.println("COMMAND A"));
commands.put("B", () -> System.out.println("COMMAND B"));
commands.put("C", () -> System.out.println("COMMAND C"));
commands.get(value).exec();

(you could use a Runnable instead of Command, but I don't consider it semantically right):

Command with one parameter:

In case you expect a parameter you could use java.util.function.Consumer:

Map<String, Consumer<Object>> commands = new HashMap<String, Consumer<Object>>();
commands.put("A", myObj::doSomethingA);
commands.put("B", myObj::doSomethingB);
commands.put("C", myObj::doSomethingC);
commands.get(value).accept(param);

In the example above, doSomethingX is a method present in myObj's class which takes any Object (named param in this example) as an argument.

Answer 10

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

Answer 11

Just use a HashMap, as described here:

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

Answer 12

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 13

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 14

Command pattern is the way to go. Here is one example using java 8:

1. Define the interface:

public interface ExtensionHandler {
  boolean isMatched(String fileName);
  String handle(String fileName);
}

2. Implement the interface with each of the extension:

public class PdfHandler implements ExtensionHandler {
  @Override
  public boolean isMatched(String fileName) {
    return fileName.endsWith(".pdf");
  }

  @Override
  public String handle(String fileName) {
    return "application/pdf";
  }
}

and

public class TxtHandler implements ExtensionHandler {
  @Override public boolean isMatched(String fileName) {
    return fileName.endsWith(".txt");
  }

  @Override public String handle(String fileName) {
    return "txt/plain";
  }
}

and so on .....

3. Define the Client:

public class MimeTypeGetter {
  private List<ExtensionHandler> extensionHandlers;
  private ExtensionHandler plainTextHandler;

  public MimeTypeGetter() {
    extensionHandlers = new ArrayList<>();

    extensionHandlers.add(new PdfHandler());
    extensionHandlers.add(new DocHandler());
    extensionHandlers.add(new XlsHandler());

    // and so on

    plainTextHandler = new PlainTextHandler();
    extensionHandlers.add(plainTextHandler);
  }

  public String getMimeType(String fileExtension) {
    return extensionHandlers.stream()
      .filter(handler -> handler.isMatched(fileExtension))
      .findFirst()
      .orElse(plainTextHandler)
      .handle(fileExtension);
  }
}

4. And this is the sample result:

  public static void main(String[] args) {
    MimeTypeGetter mimeTypeGetter = new MimeTypeGetter();

    System.out.println(mimeTypeGetter.getMimeType("test.pdf")); // application/pdf
    System.out.println(mimeTypeGetter.getMimeType("hello.txt")); // txt/plain
    System.out.println(mimeTypeGetter.getMimeType("my presentation.ppt")); // "application/vnd.ms-powerpoint"
  }

Answer 15

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...