Avoid simple if/else conditions

Question

In my program, I need to check if a variable equals 1, 2 or 3 and depending on the result, perform a different method:

if (phase.equals("1")) {
    PhaseOne.performPhase(inputParser.getSource(), inputParser.getTarget());
} else if (phase.equals("2")) {
    PhaseTwo.performPhase(inputParser.getSource(), inputParser.getTarget());
} else {
    PhaseThree.performPhase(inputParser.getSource(), inputParser.getTarget());
}

This code is so simple and basic but I really don't like it. Of course I could use switch conditions but it would, in my humble opinion, just display the same basic function in a different way.

My question is: is there a way to implement the function in an elegant and expandable way?

FYI, I already red this post but I did not find an answer which fits to my question.

Judging by the fact that your classes are named `Phase` I would suggest the State pattern: https://en.wikipedia.org/wiki/State_pattern — Jeroen Steenbeeke, Nov 13 '18 at 09:57
Are those static methods? Otherwise you can just create a `Map phases` and call `phases.get("1").performPhase()`. — Robby Cornelissen, Nov 13 '18 at 09:57
You can use [a switch](https://docs.oracle.com/javase/tutorial/java/nutsandbolts/switch.html) in the case of strings. `switch(phase) case "1": /*something*/; break; case "2" ...` — Michael, Nov 13 '18 at 09:58
You should use case instead of if, is safer because it avoids 2 situations with the same condition. Case does not alow 2 cases with the same value. — Victorqedu, Nov 13 '18 at 10:18
Related: [Replacing if else statement with pattern](https://stackoverflow.com/questions/28049094/replacing-if-else-statement-with-pattern) — LuCio, Nov 13 '18 at 11:32

Socowi · Accepted Answer · 2018-11-13T10:12:18.210

In my opinion, the accepted answer on your linked question fits you very well. Store references to the functions in the map:

Map<String,BiConsumer<T,U>> map = new HashMap<>();
map.put("1",PhaseOne::performPhase);
map.put("2",PhaseTwo::performPhase);
map.put("3",PhaseThree::performPhase);
map.get(phase).accept(inputParser.getSource(), inputParser.getTarget());

Replace T and U by the types of inputParser.getSource() and inputParser.getTarget().

With this approach, the Phase… classes don't need a common superclass or interface.

score 2 · Answer 2 · answered Nov 13 '18 at 09:57

2

If your PhaseOne/PhaseTwo/PhaseThree classes all implement the same interface (let's say Phase), and the method performPhase is defined on the interface you could do the following:

final Phase targetPhase;
switch(phase) {
    case "1": targetPhase = myInstanceOfPhaseOne; break;
    case "2": targetPhase = myInstanceOfPhaseTwo; break;
    case "3": targetPhase = myInstanceOfPhaseThree; break;
    default: throw new IllegalStateException("Unrecognised phase "+phase);
}
targetPhase.performPhase(inputParser.getSource(), inputParser.getTarget()));

answered Nov 13 '18 at 09:57

Rich

15,602
15
79
126

There are no instances. They're static methods. – Michael Nov 13 '18 at 09:59
@Michael Not necessarily. OP might also just be violating naming conventions. – Robby Cornelissen Nov 13 '18 at 10:02
@RobbyCornelissen Seems unlikely given that he's used the correct naming conventions everywhere else. – Michael Nov 13 '18 at 10:02
To be honest, this was more of a suggestion rather than a question :) – Rich Nov 13 '18 at 10:08

arxakoulini · Answer 3 · 2018-11-13T10:22:39.550

Another option is to create a Class for each Phase and an IPhase Interface for them to implement. Create a List<IPhase> with all your different Phase instances. Run a loop and if the id matches, execute the overwritten method.

public interface IPhase {
    public void performPhase();
    public String getId();
}

for (IPhase phase : phasesList){
    if (phase.equals(phase.getId())){
        phase.performPhase();
        // either break or continue the loop
    }
}

Avoid simple if/else conditions

3 Answers3