if-else structure

Question

I have these long statements that I will refer to as x,y etc. here. My conditional statements' structure goes like this:

if(x || y || z || q){
    if(x)
       do someth
    else if (y)
       do something

    if(z)
       do something
    else if(q)
       do something
}
else
    do smthing

Is there a better, shorter way to write this thing? Thanks

score 4 · Answer 1 · answered Apr 07 '10 at 07:22

4

I don't see a big problem with how you write it now. I do recommend using curly braces even for single statement if-blocks. This will help you avoid mistakes in case you have to add more code lines later (and might forget to add the curly braces then). I find it more readable as well. The code would look like this then:

if (x || y || z || q) {
    if (x) {
       do something
    } else if (y) {
       do something
    }

    if (z) {
       do something
    } else if (q) {
       do something
    }
} else {
    do something
}

answered Apr 07 '10 at 07:22

dafmetal

795
1
6
18

+1 for use curly braces. I wish Java, C++, and other languages would do what "Go" does and make those braces obligatory.... it would certainly make code a hell of a lot more readable. – Michael Aaron Safyan Apr 07 '10 at 08:09
yes and I always use curly braces actually, the one above is just a prototype or something. – Halo Apr 07 '10 at 08:22

Dr. Hans-Peter Störr · Answer 2 · 2010-04-07T09:33:48.307

4

Another variant that avoids the multiple checks and the errorprone complex logical expressions might be:

boolean conditionhandled = false;
if (x) {
   do something
   conditionhandled = true;
} else if (y) {
   do something
   conditionhandled = true;
}

if (z) {
   do something
   conditionhandled = true;
} else if (q) {
   do something
   conditionhandled = true;
}

if (!conditionhandled) {
   do something
}

edited Apr 07 '10 at 09:33

answered Apr 07 '10 at 07:24

Dr. Hans-Peter Störr

25,298
30
102
139

1

thanks, i think we need to initialize conditionhandled as false – Halo Apr 07 '10 at 07:26
Oops! Thank you, I fixed this. – Dr. Hans-Peter Störr Apr 07 '10 at 09:34

score 1 · Accepted Answer · answered Apr 07 '10 at 07:17

1

This seems pretty clear to me (and clear is good).

What you can do is first evaluate x,y,z and q and store those as variables so you don't have to do that twice.

answered Apr 07 '10 at 07:17

Thirler

20,239
14
63
92

3

@Thirler Hmm, that will defeat the purpose of short-circuit evaluation. I think on average assuming equal probability and time-complexity of x, y, z, and q this would be a performance boost. But what if the probability of z is 50% and takes little processing while the probability of q is 1% and it takes 95% of the processing power? See how micro-optimization can get you into trouble without proper metrics? – Tim Bender Apr 07 '10 at 07:30
yeah, we need to use at most one variable to do that check – Halo Apr 07 '10 at 07:35
@Tim You need extremely long conditions (or code that is executed millions of times) for this to be a performance drain. I certainly am not optimizing (the question doesn't talk about it), I'm suggesting to improve maintainability. If performance is important you will need to buffer the result of the calculation transparently (hide it behind a function that stores the result). Note that the given example executes some conditions twice. But a good rule is not to optimize at all until you see that it takes a significant amount of time to execute. – Thirler Apr 07 '10 at 07:38

uriDium · Answer 4 · 2010-04-07T11:11:23.840

1

Maybe this is a little easier to read. But now you will perform one extra check. If it is not mission critical code then maybe you can use the following:

if (x)
  do something;
else if (y)
  do something;

if (z)
  do something;
else if(q)
  do something;

if !(x || y || z || q)
  do something completely different.

edited Apr 07 '10 at 11:11

answered Apr 07 '10 at 07:18

uriDium

13,110
20
78
138

2

Your last statement isn't the same as in the question, I think you need `if !(x || y || z || q)` – Thirler Apr 07 '10 at 07:19
and using else is better i guess, for reading. You know, making sure we didn't leave holes – Halo Apr 07 '10 at 07:22
I don't see how this 'performs one extra check'. You have the same number of if statements after all :) – Tim Bender Apr 07 '10 at 07:26
@ Tim, it is an extra check because in the original if it is x, y, z it would skip to the guy's last part. In mine it would move on to the if z part. Therefore an extra check. – uriDium Apr 07 '10 at 11:14

score 0 · Answer 5 · answered Aug 11 '12 at 23:23

Can you make some assumptions about x,y,z,q? e.G. just one of them can be true. Than you could see it as a State

enum State {
X{
  void doSomething(){
    doItTheXWay();
  }  
},
Y{
  void doSomething(){
    doItTheYWay();
  }  
},
Z{
  void doSomething(){
    doItTheZWay();
  }  
},
Q{
  void doSomething(){
    doItTheQWay();
  }  
};
  void doSomething(){

  }
}

and in your code where you used the if statements

you could assign a state and just do the right thing

State state = getAState();
state.doSomething();

In case you don't like enums State could be an Interface and X to Q could be implementing classes. The benefits in this case are in multiple usage of the same if else construct. Say some codelines later you would begin with

if(x)
  do_the_next_thing_with_X();
...

or you could just extend your enum with another function and make one single call

state.doTheNextThing();

score 0 · Answer 6 · answered Apr 07 '10 at 07:22

I'm not recommending the following, in fact, I think what you got is fine, but:

s = true;
if (x) {
    do something;
    s = false;
} else if (y) {
    do something;
    s = false;
}
if (z) {
    do something;
    s = false;
} else if (q) {
    do something;
    s = false;
}

if (s) {
    so something;
}

if-else structure

6 Answers6