0

The a couple weeks ago I submit a function for code review that looked like this.

Function{
If (condition)
Else If (condition)
Else if (condition)
Else
return value
}

My lead giving the code review said "I hate else if," but he didn't say why or what I should have done instead. He just gave me the go ahead to upload this function.

My question is what are some alternatives to a bunch of "else ifs" that would make the code look more elegant and maybe perform better?

I tried pulling up his code to get an idea of what he would have done and I noticed several times he did

If (condition)
If (condition)
If (condition)

Should I avoid writing "else"? I was going to ask him but he no longer works here.

Thank you

Brian
  • 307
  • 2
  • 15
  • 4
    Did he provide any reasoning behind his statement that he "hates" `else if`? I don't see any good reason to hold that viewpoint. – Sam Estep Feb 05 '16 at 17:15
  • 2
    I would expect `else if()` to be more performant than multiple `ifs` because it will skip several condition evaluations. Then again: I can tell you right now that your performance issues won't be in this area 99.9999999% of the time – Jeroen Vannevel Feb 05 '16 at 17:16
  • Possible duplicate of [Why switch is faster than if](http://stackoverflow.com/questions/6705955/why-switch-is-faster-than-if) – sparkhee93 Feb 05 '16 at 17:19
  • You lead seems to be quite opinionated. Switch may look cleaner code, but down to the assembly level all these are jump instructions. So I don't see any reason why he should be giving you a hard time. – JVXR Feb 05 '16 at 17:21
  • I also avoid `else if` (and `switch`) on all possible places, since they are static constructs, which needs to be manually updated if more branches are necessary. Try to find a dynamic approach, but this depends on what you're trying to do with your `if/else`. – Tom Feb 05 '16 at 17:32
  • the else if will short circuit once the condition is satisfied. otherwise, each if condition will be run if the conditions are true. – shinjw Feb 05 '16 at 17:34
  • Short circuiting is why I used it. – Brian Feb 05 '16 at 17:35
  • *"he no longer works here"* - maybe because he made opinionated statements, and/or accepted code that was (in his opinion) hate-worthy? However, the code in the question is a bit toooo sketchy to profoundly say what a "better" solution could have looked like... – Marco13 Feb 05 '16 at 18:33
  • Yes everyone hated him except management, but he left for a better job. – Brian Feb 05 '16 at 18:35
  • I'm just looking for ideas to think of for the next time I'm in this situation. I like every answer I got so fat. – Brian Feb 05 '16 at 18:36
  • Well... just because their older doesn't always mean they're right. However, keep in mind that using a return; with a set of if will also be another solution for this. `if(condition) return something; if(condition) return something; if(condition) return something;` This pattern is sometimes used with factory implementations. – shinjw Feb 05 '16 at 19:02
  • *sometimes* you have `if(object is like x){process for x}else if(object is like y){process for y}` etc then you can reactor using polymorphism to `object.someMethod()` and `somemethod` is different between different (but related) types. I want to emphasis the sometimes, sometimes a line of if elses is what you want – Richard Tingle Feb 05 '16 at 19:36

5 Answers5

1

You can use switches.

switch (variable) {
  case 1:
  doSomething();
  break;
  case 2:
  doSomething();
  break;
  case 3:
  doSomething();
  break;
  default:
  doSomething();
  break;
}

https://docs.oracle.com/javase/tutorial/java/nutsandbolts/switch.html

shinjw
  • 3,329
  • 3
  • 21
  • 42
  • This doesn't chance much. It is still a static construct, like `if else`. – Tom Feb 05 '16 at 17:28
  • More information: we are forced to use Java 6 which limits what variables are used with a switch. – Brian Feb 05 '16 at 17:29
1

Oops, I misread your question, so you probably already know this, but anyway:

This is how it works:

Example 1 if your program looks like this:

if(condition1){ doThing1(); }
else if(condition2){ doThing2(); }
else if(condition3){ doThing3(); }
else{ doThing4() }
done();

Your program is first going to check if condition1 is true. if it is, it's going to run the method doThing1(). Afterwards it will not check condition2 or condition3, and will go directly to the done()-method.

If condition1 is false, the program will check if condition2 is true. If it is, it will run the method doThing2() and afterwards it will go directly to the done()-method.

If condition1 and condition2 is false, the program will check if condition3 is true and if it is, run the doThing3()-method.

If none of the conditions are true, it will run the method doThing4() and afterwards the done-method.

Example 2: However, if your program looks like this:

if(condition1){ doThing1(); }
if(condition2){ doThing2(); }
if(condition3){ doThing3(); }
done();

Your program first checks if condition1 is true, if it is, it runs the method doThing1()

Then it checks if condition2 is true, if it is, it runs the doThing2()-method

Then it checks if condition3 is true, if it is, it runs the doThing3()-method

Lastly, it runs the done()-method.

The difference is that in example 1, the program doesn't check condition2 or condition3 if condition1 is true, whereas in example 2, it always checks all of the conditions. This means that in example 1, only one of the methods doThing1(), doThing2(), doThing3() and doThing4() is going to be run. In example 2 however, if more than one of the conditions are true, the program will run more than one of the methods.

Try writing a simple program where you use the two examples and change doThing1(); to System.out.println("1"); and so on, and try out different combinations of values (true or false) for the conditions if you didn't understand my answer.

1

I try to avoid "else", "else if" and also "if" and "for". In my point of view, branching and looping add complexity to code. But it's everytime depends on what you want to do. Some examples:

If you do thing like:

if fruit.isBanana() then fruit.eatBanana()
else if fruit.isOrange() then fruit.eatOrange()
...

Instead of this, you can use inheritance:

class Banana extends Fruit {
    function eat() {
        ... yum yum yum banana ...
    }
}
class Orange extends Fruit {
    function eat() {
        ... yum yum yum orange ...
    }
}

And then, if you have an instance:

fruit.eat()

Another example: If you use "if" and "for" for filtering:

longFruits = []
for fruit in fruits {
    if fruit.isBanana() then longFruits.add(fruit)
}

then you can work with collections instead:

longFruits = CollectionUtils.select(fruits, Precicate.isBanana)

This is just a couple examples and technics.

Anton Bessonov
  • 9,208
  • 3
  • 35
  • 38
1

I would use spacing to make sure the code looks neat without changing the actual meaning of the code. you can use switch, as explained in shinjw:s answer, but that isn't always practical since you have to use a byte, short, char or int to determine the outcome. Just write

     if(condition1) //place1
else if(condition2) //place2
else if(condition3) //place3
else                //place4

to make sure the conditions are nicely lined up if you want that, but only use that if the thing you are going to write at place 1,2,3 and 4 is one line. Otherwise it looks weird. If lining up the conditions isn't that important, I would write

if(condition1)      //code1
else if(condition2) //code2
else if(condition3) //code3
else                //code4

Since Java isn't space sensitive, any place you could use one space you could use any number of spaces and anytime you could use one new line, you could use any number of blank lines. It's not the tabbing, but the {} that divides code.

1

Right up front: I like else if. I think it makes for good clean code, unless one can design away the need for a series of comparisons. That said, ...

else if is not a first-class construct in Java. It's actually an if nested inside an else. From the JLS 7 (since OP indicates they use Java 6 and I don't have the JLS for that), an IfThenElseStatement is

if ( Expression ) StatementNoShortIf else Statement

where Expression is any Java Expression that evaluates to a boolean or Boolean, and StatementNoShortIf is any Java Statement that does not end in an if with no else.

We get an else if by substituting another if statement for Statement (following the else). So

// example 1
if (condition0) {
    // body 0
} else if (condition1) {
    // body 1
} else if (condition2) {
    // body 2
} else {
    // body 3
}

is, as far as Java is concerned, identical to:

// example 2
if (condition0) {
    // body 0
} else {
    if (condition1) {
        // body 1
    } else {
        if (condition2) {
            // body 2
        } else {
            // body 3
        }
    }
}

The only difference syntactically is that in example 2 no braces have been omitted (or, put another way, nested if statements have been enclosed in a Block). The else if construct is possible solely because those enclosing Blocks are unnecessary.

It is possible that your former lead prefers to use braces wherever they can be used, thinking that this is somehow safer or more readable. I disagree with this idea: as I said, else if makes for cleaner code IMO. But the only time a series of disconnected if statements better in any sense than if else is when the conditions and associated actions are truly independent of each other.

Erick G. Hagstrom
  • 4,873
  • 1
  • 24
  • 38