Elegant way to handle multiple pathways based on compound condition

Question

Consider the following:

if (x == 5 || x == 10 || x == 12) {
    if (x == 5) {
        doSomething()
    } else if (x == 10) {
        doSomethingElse()
    } else {
        doSomeOtherThing()
    }

    doMoreThings()
}

doThingsAfterThat()

The second set of conditions feel repetitive, as I have specified them in the original condition. I could separate this into two conditionals, but then I would be repeating `doSomeOtherThings()'.

What is the most elegant way to handle this?

Edit:

This is not a duplicate of Elegant ways to handle if(if else) else. The reasons being:

In the potential duplicate question, the internal condition is testing a different value than is being tested in the original condition. In this question, the internal conditions being tested are the same as the original condition.
In this question, one has to give consideration to multiple if, else if statements. The potential duplicate question does not consider this.

possible duplicate of [Elegant ways to handle if(if else) else](http://programmers.stackexchange.com/questions/122485/elegant-ways-to-handle-ifif-else-else) — gnat, Sep 18 '15 at 04:37
ways to address issue highlighted in question edit seem to be covered in [How can I reformat my condition to make it better?](http://programmers.stackexchange.com/a/167662/31260) (truth table) and in [Style for control flow with validation checks](http://programmers.stackexchange.com/a/148855/31260) (specification pattern) — gnat, Sep 18 '15 at 07:35

score 8 · Answer 1 · answered Sep 18 '15 at 06:51

8

Procedures (or functions or methods) exist because of the need to apply the same pattern of actions in many situations. Thus there's nothing wrong with repeating a method call a few times. Repeating a call three times is not a problem.

So keep it simple, avoid needless nesting, minimize lines of code and minimize executed steps.

if (x == 5) {
    doSomething();
    doMoreThings();
} else if (x == 10) {
    doSomethingElse();
    doMoreThings();
} else if (x == 12) {
    doSomeOtherThing();
    doMoreThings();
}
doThingsAfterThat();

answered Sep 18 '15 at 06:51

COME FROM

2,666
15
15

+1 This code is much easier to read than the code in the question. – 17 of 26 Sep 18 '15 at 13:21
What's nice about this code is that, though there is a bit of duplication, it is much easier to perform extraction on should the code in the conditions get much more complicated. The duplication itself serves as a smell so that, should the code need changes, we can know that this is something to reconsider then. – cbojar Sep 18 '15 at 13:32

Mike Nakis · Answer 2 · 2015-09-18T21:45:00.900

The best solution is the one which minimizes the amount of code that you need to write, and therefore the amount of code that you need to read in order to understand what is happening. We generally read any given piece of code far more times than we write it, so clarity is the most important quality of code. (Besides correctness, of course.)

In light of this, the code in your own question is the best option.

"Come from"'s answer is quite good too, but what it boils down to is which way is more natural for you to think of the solution to the problem that your code is solving. Presumably you wrote your code your way and not "come from"'s way because that was the most natural way for you to think of solving your problem. So, that's (almost by definition I'd say) the best way.

Any other option would further complicate your code, and for what? in order to avoid writing a couple of more statements, whether they be if statements or function call statements.

Note that the fact that you write those statements twice does not necessarily mean that they will be emitted by the compiler twice, or executed twice: rest assured that the compiler will do its best to restructure the emitted code so as to prevent any unnecessary duplication. (And if it can't, then there will be a tiny bit of duplication. So what?)

Essentially, since the compiler is free to do whatever it pleases behind the scenes, the sole remaining purpose for your code is to explain what is to happen in an as clear way as possible. Conciseness is also a noble goal, but never at the expense of clarity.

I definitely agree with this view. The code should describe a solution to a problem as clearly as as possible. If for instance the condition `(x == 5 || x == 10 || x == 12)` represents an important special case, then it's probably fine to keep it visible and keep the nesting. — COME FROM, Sep 18 '15 at 12:38

score 3 · Answer 3 · answered Sep 18 '15 at 04:36

It might help if you give names to the conditions but I think it is questionable in this simple example. With more complex conditions, the benefit would be clearer. If some day you decide to change one condition, you'll only have to do so in one place and don't risk introducing an inconsistency.

final boolean someP = (x == 5);
final boolean elseP = (x == 10);
final boolean otherP = (x == 12);

if (someP) {
    doSomething();
}
if (elseP) {
    doSomethingElse();
}
if (otherP) {
    doSomeOtherThing();
}
if (someP || elseP || otherP) {
    doMoreThings();
}

doThingsAfterThat();

Manoj R · Answer 4 · 2017-01-09T11:24:43.607

why not to use case for individual if and one if for or condition.

switch (x) {
 case 5: doSomething5();
        break;
   case 10: doSomething10();
        break;
case 12 : doSomething12();
        break;
}

if (x==5 || x == 10 || x == 12){
doMoreThings();
}
doThingsAfterThat();

doMoreThings can be inside the case also. But it will be repititive.

switch (x){

case 5: doSomething5();
         doMoreThings();
        break;
case 10: doSomething10();
         doMoreThings();
        break;
case 12 : doSomething12();
         doMoreThings();
       break;
       }

*if conditions are complex then simple variable check, this will not work.

Elegant way to handle multiple pathways based on compound condition

4 Answers4