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.

Thaliathalidomide

Just a comment - I would strongly recommend picking up the Gang of Four patterns book, or if you are new to patterns, the Head First Design Patterns in Java book (which is a pretty easy read and a great introduction to a number of common patterns). Both are valuable resources, and both have saved my bacon more than once. – Edrisedrock

Yes actually I owned them but they are missing :) Thats why I was sure what I was doing was wrong :) Couldnt find a correct solution though! Maybe this gets a nice google position – Thaliathalidomide

It's just Command Pattern Monday here! – Swum

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.

Scintillation

...with the appropriate check that commandMap.get() doesn't return null :-) – Dzoba

of course, I've omitted some boilerplate code for simplicity's sake – Scintillation

Instead of a HashMap you can use a Java enum, which gives you a well defined set of commands instead of a mushy map. You could have a getter in the enum: Command getCommand(); or even implement exec() as an abstract method in the enum, which each instance implements (enum as command). – Creodont

this will force to implements all commands in the enum... that is far to be ideal. With an interface you can apply also the Decorator pattern (e.g. DebugCommandDecorator, TraceCommandDecorator), there is a lot more flexibility built-in in a simple Java interface – Scintillation

Yeah, and the HashMap also lets you add plugins at a later date. But if that's not required, and the exec() code isn't massive, you can have a single enum with all your command logic inside. Perfect for simple small sets of commands. – Creodont

Ok, for small and never growing set of commands an enum is a viable solution. – Scintillation

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.

Oppression

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.

Creodont

for each new command you add, you need to edit the switch: this code doesn't follow the open/closed principle – Scintillation

Depends on whether the commands are few, or many, doesn't it? Also this site is so appallingly slow these days it takes 5 attempts to edit an answer. – Creodont

this is not optimal see #1200146 on how to do this more optimal. – Dorseydorsiferous

Yes, thank you for spending the time to implement what I wrote at the bottom of my comment - Java Enum as Command Pattern. If I could edit my post I would mention this, but this site is dying. – Creodont

I think that this questions is screaming for a Switch statement! – Velodrome

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

Metalinguistic

+1 for mentioning delegates just in case any .NET people see this question and go crazy with the one-method interfaces. But they really aren't comparable to function pointers. They're closer to a language-supported version of the command pattern. – Serve

Answer 5

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

Waldgrave

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

Addition

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.

Dorseydorsiferous

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

Horrific

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.

Agonized

Answer 10

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

Desouza

Answer 11

Just use a HashMap, as described here:

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

Hygrothermograph

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.

Hanna

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.

Stricklin

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"
  }

Lota

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

Medan