Switch without break
Solution 1
Findbugs is flagging up that falling through from one case
to the next is generally not a good idea if there's any code in the first one (although sometimes it can be used to good effect). So when it sees the second case
and no break
, it reports the error.
So for instance:
switch (foo) {
case 0:
doSomething();
case 1:
doSomethingElse();
default:
doSomeOtherThing();
}
This is perfectly valid Java, but it probably doesn't do what the author intended: If foo
is 0
, all three of the functions doSomething
, doSomethingElse
, and doSomeOtherThing
run (in that order). If foo
is 1
, only doSomethingElse
and doSomeOtherThing
run. If foo
is any other value, only doSomeOtherThing
runs.
In contrast:
switch (foo) {
case 0:
doSomething();
break;
case 1:
doSomethingElse();
break;
default:
doSomeOtherThing();
break;
}
Here, only one of the functions will run, depending on the value of foo
.
Since it's a common coding error to forget the break
, tools like Findbugs flag it up for you.
There's a common use-case where you have multiple case
statements in a row with no intervening code:
switch (foo) {
case 0:
case 1:
doSomething();
break;
case 2:
doSomethingElse();
break;
default:
doSomeOtherThing();
break;
}
There, we want to call doSomething
if foo
is 0
or 1
. Most tools won't flag this up as a possible coding error, because there's no code in the case 0
prior to the case 1
and this is a fairly common pattern.
Solution 2
I wrote these as comments but then it's not visible. I'm turning them into an answer. This is actually an extension to T.J.Crowder's answer.
You can find the related rule that causes Findbugs to report an error here.
You can prevent Findbugs to report this kind of errors by creating an xml file with the following content, say filter.xml
and running the tool with -exclude filter.xml
option. See filters on Findbugs.
<FindBugsFilter>
<Match>
<Bug category="PERFORMANCE" />
</Match>
</FindBugsFilter>
Solution 3
Switch fall-throughs fall under the Findbugs category of "dodgy code". I think it only flags the first occurrence of a fall-through in a switch statement to cut down on the number of error messages.
Solution 4
Without the break they will fall though into each other so if x == 0
you'll go through all the code in every case statement block. Findbugs could either be wrong about the error, or it could be an error condition without the break, i.e. something in case 0
causes something in case 1
to break.
Without the exact code and error I can't really help further. Is the lack of breaks deliberate?
Solution 5
Usually bug analysis tools don't like fallthrough in code because in most cases, the user has just forgotten to write the break.
I don't know if there is a way to specifically disable the warning with FindBugs but Checkstyle tool recognize special comments such as /* fallthrough */ to assume that the user really wants the following code to be executed. Putting this kind of comment also improves readability. http://checkstyle.sourceforge.net/config_coding.html#FallThrough
Java code convention also mentions the use of fallthrough comment. http://www.oracle.com/technetwork/java/javase/documentation/codeconventions-142311.html
fastcodejava
I have been a Java programmer for a long time. Before Java, I had done some C/C++ and Fortran programming. I started an eclipse plugin project recently. I post my thoughts on technology/programming on my blog sometimes.
Updated on November 01, 2021Comments
-
fastcodejava over 2 years
I have some switch statement as shown below. Notice there is no break. Findbugs is reporting error on the second case statement only. The error is : Switch statement found where one case falls through to the next case.
switch(x) { case 0: // code case 1: // code case 2: // code }
-
T.J. Crowder over 12 yearsOn Stack Overflow, it's actually perfectly acceptable to edit someone else's answer to add useful information (such as the above). Go for it!
-
melihcelik over 12 years@T.J.Crowder unfortunately my edits were rejected, I had to undelete my answer. Thanks for the suggestion anyway.
-
kcpr over 8 yearsIs
break
really necessary at the end? -
T.J. Crowder over 8 years@kcpr: Strictly speaking, you don't need a
break;
on the last case of a switch (default
in the above). But I prefer consistency. It's purely a matter of style, the same bytecode is produced either way. (This isn't, of course, true if you leave off any otherbreak;
.) -
Kevin J. Chase about 7 yearsI suspect the actual rule was the more general
SF_SWITCH_FALLTHROUGH
.SF_DEAD_STORE_DUE_TO_SWITCH_FALLTHROUGH
means, "not only was there a fall-through, but it clobbered something you had tried to save in a previous case". That's much more likely to be a bug, in my opinion, so I would be much more hesitant to disable theDEAD_STORE
rule than theFALLTHROUGH
rule. -
Tasgall almost 7 yearsThis is a very good description of the common issue and how switch works, but doesn't really answer the OP's question - though, strictly speaking, the OP didn't actually present a question...
-
T.J. Crowder almost 7 years@Tasgall: How does it not answer the question? (Such as it is. :-) )
-
Tasgall over 6 yearsT.J.Crowder: The question - well, he didn't technically ask a question, but his situation and wording matches the question I was searching for at least :P - was how to make the findbugs tool not complain on an intentional fall through case. Your writeup on switch and why findbugs would flag it is very good and thorough, but doesn't show how to silence the warning if it was intentional. @L2M's answer below (adding "// fallthrough" or some variant where the "break;" would otherwise be) fixed it for me. Could you append that solution to your answer?
-
T.J. Crowder over 6 years@Tasgall: L2M's answer is interesting, I always include a "fallthrough" comment as a matter of style. Are you saying having the comment there makes FindBugs not report the problem?
-
Emanuel Graf over 6 yearsWhy is this logical for java? Executing a case which statement is false just because a previous case was true?
-
T.J. Crowder over 6 years@EmanuelGraf: It's just how
switch
statements traditionally work. Java inherited this from C. And note that if you have two case statements without a break, you're explicitly saying "Do this for either of the above," so it's not that you're executing code for a case that's false. It's that you're executing code for one of the two (or more) cases that you want it for. (C# modifiesswitch
to only allow stackedcase
labels if there's no code between them.) -
somejhereandthere about 5 yearsJust to add that you can avoid the
break
with other statements likereturn doSomething()
, but it supposes you are inside a function body and you don't want to proceed with the code over theswitch-case
statement. So actually a specific context.