Java Enums and Switch Statements - the default case?
Asked Answered
P

10

58

For people suggesting throwing an exception:
Throwing an exception doesn't give me a compile-time error, it gives me a runtime error. I know I can throw an exception, I'd rather die during compilation than during runtime.

First-off, I am using eclipse 3.4.

I have a data model that has a mode property that is an Enum.

enum Mode {on(...), off(...), standby(...); ...}

I am currently writing a view of this model and I have the code

...
switch(model.getMode()) {
case on:
   return getOnColor();
case off:
   return getOffColor();
case standby:
   return getStandbyColor();
}
...

I am getting an error "This method must return a result of type java.awt.Color" because I have no default case and no return xxx at the end of the function. I want a compilation error in the case where someone adds another type to the enum (e.g. shuttingdown) so I don't want to put a default case that throws an AssertionError, as this will compile with a modified Mode and not be seen as an error until runtime.

My question is this:
Why does EclipseBuilder (and javac) not recognize that this switch covers all possibilities (or does it cover them?) and stop warning me about needing a return type. Is there a way I can do what I want without adding methods to Mode?

Failing that, is there an option to warn/error on switch statements that don't cover all of the Enum's possible values?

Edit: Rob: It is a compile error. I just tried compiling it with javac and I get a "missing return statement" error targeting the last } of the method. Eclispe just places the error at the top of the method.

Polyadelphous answered 13/5, 2009 at 18:23 Comment(3)
Is the error an IDE warning or a compiler error? If the latter, then you may have few options.Chalk
Regarding wanting an error if someone changes the enum: it's nice to want things, but sometimes people want things that don't make sense. In the case of a language like Java, where files may be compiled independently, there's no way to ensure that the enum didn't get changed while you weren't looking.Nuclei
@Nuclei - However, if the enum gets changed and recompiled, so should your class be recompiled since it depends on the enum. I don't see how the ability to say "this switch handles all instances of the enum" and having the enum change to add a new instance... is any different from "this line calls the enum method blah" and having someone change the type signature of that method. Sure, Java doesn't support it now, but it could, and I don't see how it would have any negative impacts... it would just make things cleaner, easier for developers.Huffy
A
71

You could always use the Enum with Visitor pattern:

enum Mode {
  on {
      public <E> E accept( ModeVisitor<E> visitor ) {
         return visitor.visitOn();
      }
  },
  off {
      public <E> E accept( ModeVisitor<E> visitor ) {
         return visitor.visitOff();
      }
  },
  standby {
      public <E> E accept( ModeVisitor<E> visitor ) {
         return visitor.visitStandby();
      }
  }

  public abstract <E> E accept( ModeVisitor<E> visitor );

  public interface ModeVisitor<E> {
      E visitOn();
      E visitOff();
      E visitStandby();
  }
}

Then you would implement something like the following:

public final class ModeColorVisitor implements ModeVisitor<Color> {
    public Color visitOn() {
       return getOnColor();
    }

    public Color visitOff() {
       return getOffColor();
    }

    public Color visitStandby() {
       return getStandbyColor();
    }

}

You'd use it as follows:

return model.getMode().accept( new ModeColorVisitor() );

This is a lot more verbose but you'd immediately get a compile error if a new enum was declared.

Australoid answered 13/5, 2009 at 18:55 Comment(3)
This was selected because it does what I need/want. It allows arbitrary logic based on the enum state, w/o require that the enum track things that make no sense for it to track.Polyadelphous
This unfortunately doesn't work if you don't control the enum code (and can't give it an accept method).Jigger
Excellent. So useful.Sweatshop
F
56

You have to enable in Eclipse (window -> preferences) settings "Enum type constant not covered in switch" with Error level.

Throw an exception at the end of the method, but don't use default case.

public String method(Foo foo)
  switch(foo) {
  case x: return "x";
  case y: return "y";
  }

  throw new IllegalArgumentException();
}

Now if someone adds new case later, Eclipse will make him know he's missing a case. So don't ever use default unless you have really good reasons to do so.

Fimbriation answered 13/5, 2009 at 18:54 Comment(2)
I wanted to let you know the reason I accepted the other answer instead of yours. Yours work great ... as long as everyone uses Eclispe with that options set. The answer I selected works with all java compilers. If I could rate you up again I would.Polyadelphous
Thanks for commenting, I just hoped you would notice this after my editing. I forgot to include the most important part in the first draft. I'm glad I could help.Fimbriation
H
10

I don't know why you get this error, but here is a suggestion, Why don't you define the color in the enum itself? Then you can't accidentally forget to define a new color.

For example:

import java.awt.Color;

public class Test {

    enum Mode 
    {
        on (Color.BLACK), 
        off (Color.RED),
        standby (Color.GREEN);

        private final Color color; 
        Mode (Color aColor) { color = aColor; }
        Color getColor() { return color; }
    }

    class Model
    {
        private Mode mode;
        public Mode getMode () { return mode; }
    }

    private Model model;

    public Color getColor()
    {
        return model.getMode().getColor();
    }   
}

btw, for comparison here is the original case, with compiler error.

import java.awt.Color;
public class Test {

    enum Mode {on, off, standby;}

    class Model
    {
        private Mode mode;
        public Mode getMode () { return mode; }
    }

    private Model model;

    public Color getColor()
    {
        switch(model.getMode()) {
        case on:
           return Color.BLACK;
        case off:
           return Color.RED;
        case standby:
           return Color.GREEN;
        }
    }   
}
Hofstetter answered 13/5, 2009 at 18:49 Comment(0)
S
6

I'd say it's probably because model.GetMode() could return null.

Saltwort answered 13/5, 2009 at 18:30 Comment(4)
yup. enums are refence types.Overfill
I just checked. I had assumed Sun knew wtf they were doing with Enums, but apparently you can assign null to an enum type. Whose braindead idea was that. Thanks for pointing this out.Polyadelphous
That doesn't make sense. That would lead to a NullPointerException anyway. You can't define a "case null"Hofstetter
While enum variables are nullable, those null values can't be used in a switch (they result in a NullPointerException). This is certainly not the reason for this.Jigger
U
3

A nice way for this would be to add the default case to return some error value or throw exception and to use automated tests with jUnit, for example:

@Test
public void testEnum() {
  for(Mode m : Mode.values() {
    m.foobar(); // The switch is separated to a method
    // If you want to check the return value, do it (or if there's an exception in the 
    // default part, that's enough)
  }
}

When you got automated tests, this will take care of that foobar is defined for all enumerations.

Unhopedfor answered 15/5, 2009 at 11:56 Comment(0)
N
2

Create a default case that throws an exception:

throw new RuntimeExeption("this code should never be hit unless someone updated the enum") 

... and that pretty much describes why Eclipse is complaining: while your switch may cover all enum cases today, someone could add a case and not recompile tomorrow.

Nuclei answered 13/5, 2009 at 18:26 Comment(0)
S
2

Why does EclipseBuilder not recognize that this switch covers all possibilities (or does it cover them?) and stop warning me about needing a return type. Is there a way I can do what I want without adding methods to Mode?

It's not an issue in Eclipse, but rather the compiler, javac. All javac sees is that you don't have a return value in the case in which nothing is matched (the fact that you know you are matching all cases is irrelevant). You have to return something in the default case (or throw an exception).

Personally, I'd just throw some sort of exception.

Sentient answered 13/5, 2009 at 18:27 Comment(3)
I know this is nitpicky, but Eclipse doesn't use javac. It has its own internal Java compiler.Extrinsic
You'll notice that I specified EclipseBuilder as generating the error. Also, throwing an exception doesn't give me a compile-time error, it gives me a runtime error, which is what I though Enums were supposed to provide (compared to using ints like c-enums).Polyadelphous
Little known fact: the Eclipse JDT actually include their own compiler separte from javac. eclipse.org/jdt/core/index.phpOverfill
A
2

Your problem is that you are trying to use the switch statement as an indicator that your enum is locked down.

The fact is that the 'switch' statement and the java compiler cannot recognize that you do not want to allow other options in your enum. The fact that you only want three options in your enum is completely separate from your design of the switch statement, which as noted by others should ALWAYS have a default statement. (In your case it should throw an exception, because it is an unhandled scenario.)

You should liberally sprinkle your enum with comments so that everyone knows not to touch it, and you should fix your switch statement to throw errors for unrecognized cases. That way you've covered all the bases.

EDIT

On the matter of throwing compiler error. That does not strictly make sense. You have an enum with three options, and a switch with three options. You want it to throw a compiler error if someone adds a value to the enum. Except that enums can be of any size, so it doesn't make sense to throw a compiler error if someone changes it. Furthermore, you are defining the size of your enum based on a switch statement which could be located in a completely different class.

The internal workings of Enum and Switch are completely separate and should remain uncoupled.

Adlay answered 13/5, 2009 at 18:48 Comment(0)
D
0

Since I can't just comment...

  1. Always, Always, Always have a default case. You'd be surprised how "frequently" it would be hit (Less in Java than C, but still).

  2. Having said that, what if I only want to handle only on/off in my case. Your semantic processing by javac would flag that as an issue.

Dewy answered 13/5, 2009 at 18:41 Comment(1)
For (2) you would have a default case covering enums you're not handling. I think the issue is that he's handling every enum already, so he shouldn't get a "no return value" error during compile.Unconcern
J
0

Nowadays (this answer is written several years after the original question), eclipse allows following configuration at Window -> Preferences -> Java -> Compiler -> Error/warnings -> Potential programming problems:

Incomplete switch cases

Signal even if default case exists

Juggernaut answered 28/12, 2016 at 15:50 Comment(0)

© 2022 - 2024 — McMap. All rights reserved.