Avoid Switch Fallthrough
| | class BoardComputer { |
| | |
| | CruiseControl cruiseControl; |
| | |
| | void authorize(User user) { |
| | Objects.requireNonNull(user); |
| | switch (user.getRank()) { |
| | case UNKNOWN: |
| » | cruiseControl.logUnauthorizedAccessAttempt(); |
| | case ASTRONAUT: |
| | cruiseControl.grantAccess(user); |
| | break; |
| | case COMMANDER: |
| | cruiseControl.grantAccess(user); |
| | cruiseControl.grantAdminAccess(user); |
| | break; |
| | } |
| | } |
| | } |
Some programming language constructs are infamous because of all the bugs they’ve caused over the years. One of them is the switch, and you should be careful when you use it.
The method authorizeUser() you see here validates its parameters and checks for a null reference. It uses Objects.requireNonNull(), a handy parameter validation method from the Java API, which triggers an exception for null input. But authorizeUser() still contains a classic bug: the switch fallthrough.
The bug is in the first case of the switch statement. There’s no break statement at the end of the case. This means that the switch just continues to the next case—it falls through and will always execute cruiseControl.grantAccess().
The switch statement is infamous for this behavior. It always continues execution until it reaches a break statement or the end of the block. You’re going to find these execution semantics for the switch in any C-style language. Also, Java inherited it from C.
The switch behaves the way it does because this allows you to write code that’s a little more compact and requires fewer characters, you can avoid evaluating a few condition expressions this way. And since Java 7, the switch statement even supports strings in addition to integers, characters, and enums.
But over the years, it has done more harm than good, and quite a few bugs resulted from forgotten break statements. In the rare case where a missing break is intentional, you should leave a comment!
Take a look at a bug-free version of the code.
| | class BoardComputer { |
| | |
| | CruiseControl cruiseControl; |
| | |
| | void authorize(User user) { |
| | Objects.requireNonNull(user); |
| | switch (user.getRank()) { |
| | case UNKNOWN: |
| | cruiseControl.logUnauthorizedAccessAttempt(); |
| » | break; |
| | case ASTRONAUT: |
| | cruiseControl.grantAccess(user); |
| | break; |
| | case COMMANDER: |
| | cruiseControl.grantAccess(user); |
| | cruiseControl.grantAdminAccess(user); |
| | break; |
| | } |
| | } |
| | } |
It’s easy to fix the unintentional switch fallthrough: make sure that there’s a break statement at the end of every case. We’ve done this in our code example.
Now the code is bug free—but is the switch really the perfect choice here? Nope!
In the example, the switch mixes two different concerns that should be separate: It combines unauthorized access and authorized access into one block of code.
As a rule of thumb, you should put different concerns into different blocks of code (we’ll look at that more closely in Ensure Code Symmetry). First of all, you’ll make the code more readable that way. And second, there’s a smaller chance of accidental bugs like the switch fallthrough.
You’ll find it hard to achieve this separation of concerns with the switch. That’s why we prefer using if statements instead. But even there, the switch fallthrough can haunt us, as you’ll see in Always Use Braces.
What if one adds another rank? You would have to adapt this conditional, but that’s easy to forget. The code would still run through, and you wouldn’t even notice that there’s something missing! That’s why you should always have a fallback branch that captures values that were not explicitly coded. switch statements have this built in with the default case. Alternatively, you can throw an AssertionError to be sure.