What is a good "unreachable" Error for switches?

Eclipse forces me to use a default case for any switch including those listing all declared enum values, allegedly because of the language specification [1]. This is unfortunate because Android Studio, in which the project is developed in parallel, does not, and would naturally warn me about all switches which become incomplete if ever the enum is changed. While I would prefer the latter behaviour because the former makes enum changes actually more error-prone (see example below), I am in no position to choose, so I need to find how to do this right for both. I suppose if there is a place in a code which should under any circumstances remain unreachable but still removing the line is not an option, throwing an Error seems like the natural thing to do there. But which one, is there a generally accepted subclass for such a scenario (perhaps extending to other "forced" unreachable places)? Or is it acceptable to simply throw a quick-and-dirty new Error("enum name") just for the sake of it, instead of writing my own just to be never used?

In the example:

public static enum Color {
    RED,
    GREEN,
    BLUE;

    @Override
    public String toString() {
        switch(this) {
            case RED:
                return "Red";
            case GREEN:
                return "Green";
            case BLUE:
                return "Blue";
            default:
                /* never reached */
                throw new ThisCanNeverHappenError();
        }
    }
}

adding WHITE to the enum makes this switch and possibly many more throughout the code silent sources of nasty errors as flow control finds them to be just fine.

Answers


You should not throw an Error. A better exception should be IllegalStateException:

switch(this) {
    case RED:
        return "Red";
    case GREEN:
        return "Green";
    case BLUE:
        return "Blue";
    default:
        throw new IllegalStateException("Unexpected enum value: " + this.name());
}

On a different note, you shouldn't use a switch statement there anyway. Add a field to the enum. Also note that enums are always static, so you can remove that keyword.

public enum Color {
    RED  ("Red"),
    GREEN("Green"),
    BLUE ("Blue");

    private final String displayText;

    private Color(String displayText) {
        this.displayText = displayText;
    }
    public String getDisplayText() {
        return this.displayText;
    }
    @Override
    public String toString() {
        return this.displayText;
    }
}

There was a LONG discussion of this in the Eclipse Bugzilla/Bug 374605.

The end result is that this is a configurable warning and can be disabled.

Change the dropdown from Warning to Ignore


There is no need for this switch (and the corresponding handling of the default case) if you add the "toString" value as a parameter of the enum instances:

public static enum Color {
    RED("Red"),
    GREEN("Green"),
    BLUE("Blue");

    private final String name;

    private Color(String name) {
      this.name = name;
    }

    @Override
    public String toString() {
      return name;
    }
}

Throwing Errors is not a very wise move - most exception-handling code passes them thru, they end up in strange places, might make your process unusable.

I would recommend IllegalStateException.


You are making an assertion: /* never reached */ - if that assertion is erroneous you should probably throw an AssertionError.

The alternative recommendation of throwing an IllegalStateException is not really appropriate - according to the javadoc:

Signals that a method has been invoked at an illegal or inappropriate time

which is not really the case here.


Need Your Help

LINQ contains in a list within a list

c# linq

I'm trying to use a contains in a list within a list but been stuck on this one: