Eclipse Enum Switch Warning
I recently encountered some Java code with what seemed to be an odd warning. The code in question was a switch statement that used an enumeration to make its selection. Below is a trivial example.
public class EnumSwitch { enum Count { ONE, TWO, OTHER } public int toNumber(final Count count) { switch (count) { case ONE: return 1; case TWO: return 2; } return -1; } }
In our simple example, we’ve got an enumeration with four potential values. There’s also a trivial method to turn the values into a number. Certainly there are better ways to do this, but that’s not the point of this discussion.
At first glance it seems to be a perfectly valid switch statement. If the passed value matches one of the cases, the appropriate body for the matching case will be executed, which in this example simply returns a number.
Eclipse disagreed and warned that there needs to be a case for the Count.OTHER enumeration value.
It should be the case that if the switch doesn’t contain a matching case the body of the switch will be skipped, and indeed it does function this way. This can be tested by adding the following method to the class and debugging it–all of the asserts will pass, so the switch functions as expected.
public static void main(final String[] args) { final EnumSwitch enumSwitch = new EnumSwitch(); assert(1 == enumSwitch.toNumber(Count.ONE)); assert(2 == enumSwitch.toNumber(Count.TWO)); assert(-1 == enumSwitch.toNumber(Count.OTHER)); assert(-1 == enumSwitch.toNumber(null)); }
The warning then left me scratching my head. It turns out that to remove the warning an easy solution is to let Eclipse add each enumeration member or a default case to the switch (or do it manually, of course). Depending on the style of the developer, it might be that “default: return -1;” would be preferred to having a return after the switch. Of course, that totally depends on the method as it may not always be the case that the method ends with the end of the switch statement.
Despite having such a simple solution, I wanted to know what compelled the Eclipse folks to add such a warning.
I found in the Eclipse Help that there is a mention in the section titled “Handling all enum constants” that it may be desirable to include all possible enumeration elements, or add a default, or disable the warning. This explained why Eclipse was doing it, and why the simple solution worked, but not why it was desired.
A little more searching led to a discussion citing the Java Language Specification note that specifically addresses this case. In JSL 14.11 there’s a note:
Compilers are encouraged (but not required) to provide a warning if a switch on an enum-valued expression lacks a default case and lacks cases for one or more of the enum type’s constants. (Such a statement will silently do nothing if the expression evaluates to one of the missing constants.)
Curiously it seems that in the special situation where using a switch to select an action based on an enumeration that Java wants developers to be more explicit; of course it’s harder to determine if there are cases left out when not using an enumeration, such as when using String or numbers. Although the comment in the JLS also says that the switch will do nothing when there are missing constants, it seems that it also still encourages compilers to make a warning.
Note that while it’s in the JSL I could only get Eclipse to give me the warning. Compiling the class using the command-line compiler from Oracle or using either NetBeans or IntelliJ. The warning seems unique to Eclipse. I’m not sure if it’s a recent addition or if I’ve only just now encountered it.
Why is this a deal at all? The code on which I noticed the error represented a valid case where not all of the values of the enumeration had any usefulness in the switch. Here’s a tweak to the class that shows this example (again, not the best way to do it, but fair for the example):
public class EnumSwitch { enum Planet { MERCURY,VENUS,EARTH,MARS, JUPITER,SATURN,URANUS,NEPTUNE } public boolean isInner(final Planet planet) { switch (planet) { case MERCURY: case VENUS: case EARTH: case MARS: return true; } return false; } }
In this case, and in a presumably isOuter() method or a hasMoons() method, only some of the enumeration values are useful. Adding all of them would be arguably unnecessarily verbose, and could potentially generate code that gets difficult to read. Again adding a default and possibly moving the “return false” within the switch would satisfy the compiler, and still would remain terse.
The important thing the example shows, however trivially, is that it could become cumbersome to include a case for every member of a larger enumeration. Additionally, it could become an issue of maintenance if the enumeration were changed, as all of the switch statements that used the enumeration (that didn’t have a default) would need to be updated.
The most simple solution to all of this is to add an empty default to the end of every switch. It satisfies the intent of the JSL and suppresses the Eclipse warning. It will also prepare code for a future where compilers follow the JSL more strictly and start complaining about all of the enumeration switches that are missing enumeration values.
I would probably have written…
public class EnumSwitch {
enum Count { ONE, TWO, OTHER }
public int toNumber(final Count count) {
switch (count) {
case ONE: return 1;
case TWO: return 2;
default: return -1;
}
}
}
… in the first place, just because parallel construction is more readable than non-parallel. In cases where it would seem that “the method does not end with the end of the switch statement” but all the enumerated cases in the switch statement end with returns, I generally refactor the remainder of the method into another method call and have the default case return the result of THAT method:
…
default: return theRestOfTheStory(p1, p2, p3);
But it’s nice to know about JSL 14.11 in any case.
Thanks!
The example is pretty trivial, and shows that the method is returning when a switch case is met. In the real program, as with many more instances of switch, it’s doing some decision-making and work in the middle of the method, and doesn’t necessarily return.
Here’s a simple example of that kind of implementation:
enum Thing { THING, OTHER, MORE };
public void foo(){
Thing thing = someWorkForValue();
switch(thing){
case THING: doThing(); break;
case OTHER: doOther(); break;
}
keepWorking();
}