2

I was just wondering which of these is faster to check the int values and set the boolean to the right value?

switch (type) {
    case INCREASING: 
        if (currentVal >= duration) { done = true; }
        break;
    case DECREASING:
        if (currentVal <= 0) { done = true; }
        break;
    default: 
        done = false;
        break;
    }

or

done = (type == INCREASING ? currentVal >= duration ? true : false : false) || (type == DECREASING ? currentVal <= 0 ? true : false : false);

with 

public static final int INCREASING = 1;
public static final int DECREASING = -1;
private float currentVal;
private float duration; //is set in the constructur
private boolean done = false;

They both do the same in terms of what i want to achieve with it. I just thought the switch statement might be a little faster because it doesn't check everything? I like the advantage of having it in one line though so is the difference actually worth considering?

user2864740
  • 60,010
  • 15
  • 145
  • 220
cozmic
  • 178
  • 2
  • 7
  • 8
    the switch statement is definitely more readable. – Nivas Sep 10 '14 at 20:47
  • 8
    Don't prematurely optimize your code. Use the more readable solution. – clcto Sep 10 '14 at 20:48
  • Readability, I would go with the first one. You want others to be able to understand what you are doing too. The question I always ask myself is 5 years later when I look back on this project, will I be able to understand what I wrote. If you're answer is no (you're not being clear) – Moonhead Sep 10 '14 at 20:48
  • 1
    Java lookupswitch command is supposedly O(log n), instead of O(n) for the naive ternary. – jn1kk Sep 10 '14 at 20:55
  • actually, the time of switch lookup is dependent on the data; in simple cases, you can assume they are just equal to each other (see Dr Google for specifics); also, it's a duplicate of http://stackoverflow.com/questions/9745389/is-the-ternary-operator-faster-than-an-if-condition?lq=1 –  Sep 10 '14 at 20:58
  • I seriously doubt, there is any measurable performance difference between these two statements in any real world usage. So from these two I would go with the switch, if you don't want to use any of the "short circuit" expressions propossed in the answers. – Mikkel Løkke Sep 10 '14 at 20:58
  • switch style could be the best option if there's going to be more than just 2 types in the enum - but I'd still go with `case INCREASING: done = (currentVal >= duration); break;` style instead of what's currently up there; setting boolean based on condition is always less readable for experienced programmer than setting the boolean **to** condition directly. –  Sep 10 '14 at 21:05
  • 1
    I'm going to leave this open (and not close as a duplicate) because the `switch` makes it distinct from the cited duplicate. From my compiler theory class, I know switches can be handled differently then `if/then/else` blocks. For example, a jump table could be used in a dense switch. – jww Sep 10 '14 at 21:39

4 Answers4

10

A third option:

done = (type == INCREASING && currentVal >= duration) ||
       (type == DECREASING && currentVal <= 0);

I think it's a pretty good compromise between brevity and readability. As others have mentioned, the speed is pretty irrelevant (in the worst case you're doing four integer comparisons and three boolean comparisons) unless, after you've put this code into production, you see performance issues and are able to determine that there's a bottleneck here.

Jordan Running
  • 102,619
  • 17
  • 182
  • 182
  • 1
    +1 for the parentheses around both conditions; they not only make the code more readable, but also prevent associativity/OoP errors. another mental +1 for mentioning that premature is a bad thing, always. Makes the fun shorter IMO *chuckle* –  Sep 10 '14 at 21:01
1

There should not be a major difference in speed. However, the switch is far more readable and maintainable.

  • Easy to read and understand
  • Adding a new option would be very easier in the switch
  • The switch option is easier to debug.

See also Is the ternary operator faster than an “if” condition

Community
  • 1
  • 1
Nivas
  • 18,126
  • 4
  • 62
  • 76
1

The switch statement often use an equivalent of hash code/equals (at least for String and Enum) or a jump table for other case, and your code:

done = ( type == INCREASING ? 
          currentVal >= duration ? true : false  // A
          : false) // B
    || ( type == DECREASING ? 
          currentVal <= 0 ? true : false  // C
       : false ) // D;

has a lazy evaluation:

  • type == INCREASING => true, don't execute D.
  • type == DECREASING => false, don't execute C.
  • a || b don't execute b if a is true
  • a && b don't execute b if a is false

And you should write it like this:

done = ( (type == INCREASING) ? currentVal >= duration : false) // B
    || ( (type == DECREASING) ? currentVal <= 0 : false ) // D;

And then:

done = (type == INCREASING && currentVal >= duration)
    || (type == DECREASING && currentVal <= 0);

I would stick to the switch, or to simple if/else, if it gets more complicated than the above expression rather than using imbrications of ternary operator which is far from being readable (I used new lines to get it more readable in the first example).

NoDataFound
  • 11,381
  • 33
  • 59
  • I've added the parentheses since there are numerous problems with ternary without parentheses; same goes with `||` / `&&` (operator precedence and LTR/RTL associativity is a bitch; don't provoke unless necessary) –  Sep 10 '14 at 21:00
  • I deleted my comments since I could not edit the first and the second would be irrelevant without the first. I don't know why someone down voted me, but I hope it's not because the first expression in my answer does not come with parenthesis between each part because it is a plain copy/paste of the question. – NoDataFound Sep 11 '14 at 19:23
  • And for the parenthesis, I'd better explain my opinion: I don't mean it like _you should always create variable to express a part of a complex expression_ but _you should express the meaning of part of your complex expression_. Variables are a way to do it, comments another way (like how I put `// D` in my answer). Complex formula lacking indentation/comments/variables like your ideone example (or Excel formula!) are examples of inherently unreadable expression and absence of parenthesis only make it worse with tricky/less used operators (like `<<`, `>>`, etc). – NoDataFound Sep 11 '14 at 19:34
  • @maaartinus The problem with this reasoning is that all langs (with the notable exception of Whitespace) *whitespace is a semantically and syntactically void element*; they never change the meaning, so it's only *stylistic change*. On the other hand, *parentheses have both semantic and syntactic meaning* - placing them *may* and probably *will* change the code's behaviour. As such, I'm liberal when it comes to strictly visual rules/changes - irregularly used space will hurt only the eyes. OTOH, playing with parentheses is like playing with fire - wrong style rules == incoming trouble. –  Sep 11 '14 at 21:05
1

This is not an "answer"; but just a viewpoint not able to fit in a comment.

I would likely have written it like this:

if (type == INCREASING && currentVal >= duration) {
    done = true;
} else if (type == DECREASING && currentVal <= 0) {
    done = true;
}

There is no else / done = false, because if cleared that is like a concern elsewhere. Also done = true might be better replaced with a break or return, etc.

user2864740
  • 60,010
  • 15
  • 145
  • 220