Regarding Java switch statements - using return and omitting breaks in each case

Question

Given this method, does this represent some egregious stylistic or semantic faux pas:

private double translateSlider(int sliderVal) {
    switch (sliderVal) {
        case 0:
            return 1.0;
        case 1:
            return .9;
        case 2:
            return .8;
        case 3:
            return .7;
        case 4:
            return .6;
        default:
            return 1.0;
    }
}

It's clearly not in line with the Java tutorials here.

However, It's clear, concise and so far has yielded exactly what I need. Is there a compelling, pragmatic reason to create a local variable, assign a value to it within each case, add a break to each case and return the value at the end of the method?

This looks fine to me. The switch statement is the only content of the method, and it's clear and readable, so that makes perfect sense. — Michelle, Aug 12 '13 at 16:41
Prefer clear and simple code, vs clever code. I spent 1 second to immediately understand what the OP's method does, then i stare at AlexWien 's method for 1 minute and my brain simply refuse to decipher it. Amen. — jumping_monkey, Jun 08 '22 at 02:15

score 88 · Accepted Answer · edited May 23 '17 at 12:02

88

Assigning a value to a local variable and then returning that at the end is considered a good practice. Methods having multiple exits are harder to debug and can be difficult to read.

That said, thats the only plus point left to this paradigm. It was originated when only low-level procedural languages were around. And it made much more sense at that time.

While we are on the topic you must check this out. Its an interesting read.

edited May 23 '17 at 12:02

Community

1
1

answered Aug 12 '13 at 16:42

rocketboy

9,573
2
34
36

15

Links an SO post and says that multiple exits are bad. Top voted answer by far on that question starts with "it largely comes down to personal preference" and ends with "enforcing a single exit point is a pointless or even counterproductive restriction IMHO". -_- *long sigh* – Millie Smith Jan 28 '16 at 08:20
@MillieSmith Neither or things are said in either of the top voted answers. Did I miss something? – Ungeheuer Nov 21 '16 at 05:07
@Adrian The [question he linked](http://stackoverflow.com/q/4838828/2850543) and the [answer I'm referring to](http://stackoverflow.com/a/4838892/2850543). – Millie Smith Nov 21 '16 at 07:45
@MillieSmith oh, i clicked on a different link. explains why I didn't see it lol – Ungeheuer Nov 21 '16 at 17:19
@MillieSmith The user points out all the different schools of thought on the topic and states his opinion at the end, seems alright. – rocketboy May 16 '18 at 05:39
With the techniques and tools we are using today "harder to debug" argument becomes wrong. – v.ladynev Oct 28 '22 at 12:52

score 9 · Answer 2 · answered Sep 12 '22 at 17:23

9

If it's Java 14+, you may use functional expression of switch like this:

return switch(region) {
    case "us-east-1" -> Region.US_EAST_1;
    default -> Region.US_EAST_1;
};

answered Sep 12 '22 at 17:23

Tony Puthenveettil

366
3
4

score 7 · Answer 3 · edited Aug 05 '20 at 19:48

7

From human intelligence view your code is fine. From static code analysis tools view there are multiple returns, which makes it harder to debug. e.g you cannot set one and only breakpoint immediately before return.

Further you would not hard code the 4 slider steps in an professional app. Either calculate the values by using max - min, etc., or look them up in an array:

public static final double[] SLIDER_VALUES = {1.0, 0.9, 0.8, 0.7, 0.6};
public static final double SLIDER_DEFAULT = 1.0;


private double translateSlider(int sliderValue) {
  double result = SLIDER_DEFAULT;
  if (sliderValue >= 0 && sliderValue < SLIDER_VALUES.length) {
      ret = SLIDER_VALUES[sliderValue];
  }

  return result;
}

edited Aug 05 '20 at 19:48

informatik01

16,038
10
74
104

answered Aug 12 '13 at 16:46

AlexWien

28,470
6
53
83

There is a downside to using an array here. The indices in the array implicitly correspond to possible slider input values. But it's very implicit. What happens if someone decides to make the slider start at 1 or move in increments of 5? It might be more flexible in the long term to use a map (HashMap) from slider value to the output value. Either way this becomes a faster operation than hopping through a switch statement, so good point – Andrew Puglionesi Sep 06 '19 at 19:40

score 3 · Answer 4 · edited Aug 04 '19 at 14:57

I think that what you have written is perfectly fine. I also don't see any readability issue with having multiple return statements.

I would always prefer to return from the point in the code when I know to return and this will avoid running logic below the return.

There can be an argument for having a single return point for debugging and logging. But, in your code, there is no issue of debugging and logging if we use it. It is very simple and readable the way you wrote.

Dwight Spencer · Answer 5 · 2020-10-16T05:16:53.170

Best case for human logic to computer generated bytecode would be to utilize code like the following:

private double translateSlider(int sliderVal) {
  float retval = 1.0;

  switch (sliderVal) {
    case 1: retval = 0.9; break;
    case 2: retval = 0.8; break;
    case 3: retval = 0.7; break;
    case 4: retval = 0.6; break;
    case 0:
    default: break;
  }
  return retval;
}

Thus eliminating multiple exits from the method and utilizing the language logically. (ie while sliderVal is an integer range of 1-4 change float value else if sliderVal is 0 and all other values, retval stays the same float value of 1.0)

However something like this with each integer value of sliderVal being (n-(n/10)) one really could just do a lambda and get a faster results:

private double translateSlider = (int sliderVal) -> (1.0-(siderVal/10));

Edit: A modulus of 4 may be in order to keep logic (ie (n-(n/10))%4))

You need to add `break;` to each `case` above — currently 1–4 will all fall through to `retval = 0.6`. — stevek_mcc, Aug 29 '18 at 13:27
Wow, 2015.. Definitely have grown as a developer. Looking back on this code; one starts to think, "why didn't that just get implemented as a lookup table with a hashmap." — Dwight Spencer, Oct 16 '20 at 05:19

score 0 · Answer 6 · answered Aug 12 '13 at 16:43

0

Nope, what you have is fine. You could also do this as a formula (sliderVal < 5 ? (1.0 - 0.1 * sliderVal) : 1.0) or use a Map<Integer,Double>, but what you have is fine.

answered Aug 12 '13 at 16:43

yshavit

42,327
7
87
124

3

This won't give the same return value as in case of OP code. `0.1` doesn't have exact binary representation, and by multiplying it with `sliderVal`, you are compounding the precision error. – Rohit Jain Aug 12 '13 at 16:44
@RohitJain In the general case, sure; but in this case, you get you the same values for `0 <= i <= 10`: http://ideone.com/3F9y8K. If the OP is working with doubles anyway, there's a good chance there'll be other rounding errors to deal with, as those are par for the course. – yshavit Aug 12 '13 at 16:55
the ? operator is discouraged: Your propsoded solution solves the multiple return warnuings, but triggers the ? operator warning – AlexWien Aug 12 '13 at 17:06
3

@AlexWien "Is discouraged"? Don't make this sound like an absolute rule; there are plenty of people who are just fine with an unchained ternary operator. If you don't like it, do the trivial expansion to the if-else in your head; my main point was that this problem can be solved without brute strengthing the 6 options. – yshavit Aug 12 '13 at 17:16
The map is a bad idea. Shooting with big cannons to small birds (probably not well translated) is not necesarry here. a simple fixed array is sufficient. – AlexWien Aug 12 '13 at 17:29

score 0 · Answer 7 · answered Aug 12 '13 at 16:43

0

I suggest you not use literals.

Other than that the style itself looks fine.

answered Aug 12 '13 at 16:43

William Morrison

10,953
2
31
48

Where's that comparison? The switch is on an int. – yshavit Aug 12 '13 at 16:43
In my own deluded mind :) misread, thought the double return was a `case` statement. – William Morrison Aug 12 '13 at 16:44

score 0 · Answer 8 · answered Aug 12 '13 at 16:43

If you're going to have a method that just runs the switch and then returns some value, then sure this way works. But if you want a switch with other stuff in a method then you can't use return or the rest of the code inside the method will not execute. Notice in the tutorial how it has a print after the code? Yours would not be able to do this.

SubSevn · Answer 9 · 2013-08-12T16:51:21.813

0

Why not just

private double translateSlider(int sliderval) {
if(sliderval > 4 || sliderval < 0)
    return 1.0d;
return (1.0d - ((double)sliderval/10.0d));
}

Or similar?

edited Aug 12 '13 at 16:51

answered Aug 12 '13 at 16:44

SubSevn

1,008
2
10
27

1

This isn't as computationally "quick" as your solution, so if you're looking to save cycles, floating point ops might not be as good of a choice - or if you're looking for more "configuration" (that is, the maths won't always work out this well). – SubSevn Aug 12 '13 at 16:45
@SubSeven there is never need to save cylces when a user moves a slider with his slow fingers. even the slowest embedded device outperforms his finger by far. readibility and clearness of code is the maxime. – AlexWien Aug 12 '13 at 17:05
The hard coded 4, triggers yet another warning (magic numbers) – AlexWien Aug 12 '13 at 17:15
shouldn't that be if (sliderVal > 0 && sliderVal <= 4) retval ::= 1.0 - sliderVal/10) else retval ::= 1.0? – Dwight Spencer Jun 30 '15 at 20:06

score 0 · Answer 10 · answered Dec 12 '20 at 17:12

0

Though the question is old enough it still can be referenced nowdays.

Semantically that is exactly what Java 12 introduced (https://openjdk.java.net/jeps/325), thus, exactly in that simple example provided I can't see any problem or cons.

answered Dec 12 '20 at 17:12

humkins

9,635
11
57
75

score -2 · Answer 11 · answered Aug 12 '13 at 16:44

-2

Yes this is good. Tutorials are not always consize and neat. Not only that, creating local variables is waste of space and inefficient

answered Aug 12 '13 at 16:44

Xperiaz X

216
1
6
16

4

IDK where you learnt such hackery but local varables are always removed by the Garbage Collector and only stay in context of the method as its called therefor never inefficient or a "waste of space". – Dwight Spencer Jun 30 '15 at 20:09

Regarding Java switch statements - using return and omitting breaks in each case

11 Answers11

Linked