Ensure every enum value is used

Question

If I´m using an enum to determine the type of a task.

public enum TaskType {
    TYPE_ONE("Type1"),TYPE_TWO("Type2"),TYPE_THREE("Type3");

    private final String type;

    private StageType(String type) {
        this.type = type;
    }

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

how can I assure at one point in my Application

if(taskType == TaskType.TYPE_ONE) {
    typeOneProcessing();
} else if(taskType == TaskType.TYPE_TWO) {
    typeTwoProcessing();
} else if(taskType == TaskType.TYPE_THREE) {
    typeThreeProcessing();
}

that every enum value is used?
I mean if I need to add a new TYPE_FOUR someday, I´d need to find every place in my code where I used the enum, so I ask myself if there is a better way so that I either avoid the enum and use some other concept or that I can ensure that every value of the enum is used in that piece of code.

Can we assume that each `typeXProcessing` will be the same everywhere? If so maybe it would be better to put it inside each type and invoke something like `taskType.process()`. — Pshemo, Mar 12 '15 at 11:35

OldCurmudgeon · Answer 1 · 2015-03-12T11:45:30.057

There are findbugs type tools for doing that but you could consider removing the if-then-else completely and put the processing inside the enum. Here, adding a new TYPE_FOUR will force you to write it's doProcessing() method.

public interface DoesProcessing {

    public void doProcessing();
}

public enum TaskType implements DoesProcessing {

    TYPE_ONE("Type1") {
                @Override
                public void doProcessing() {

                }
            },
    TYPE_TWO("Type2") {
                @Override
                public void doProcessing() {

                }
            },
    TYPE_THREE("Type3") {
                @Override
                public void doProcessing() {

                }
            },
    TYPE_FOUR("Type4") {
        // error: <anonymous com.oldcurmudgeon.test.Test$TaskType$4> is not abstract and does not override abstract method doProcessing() in DoesProcessing
            };

    private final String type;

    private TaskType(String type) {
        this.type = type;
    }

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

public void test() {
    DoesProcessing type = TaskType.TYPE_TWO;
    type.doProcessing();
}

If you would prefer an abstract method then this works:

public enum TaskType {

    TYPE_ONE("Type1") {
                @Override
                public void doProcessing() {

                }
            },
    TYPE_TWO("Type2") {
                @Override
                public void doProcessing() {

                }
            },
    TYPE_THREE("Type3") {
                @Override
                public void doProcessing() {

                }
            };

    private final String type;

    private TaskType(String type) {
        this.type = type;
    }

    // Force them all to implement doProcessing.
    public abstract void doProcessing();

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

Why you don't simply add an abstract method in the enum instead of this interface? — Alexis C., Mar 12 '15 at 11:42
@AlexisC. - Could do that too but implementing an interface is more flexible - and other enums can implement it too. — OldCurmudgeon, Mar 12 '15 at 11:43
This is a really good solution. This is what I ended up doing back when I had the same problem as the question asker (back on a question http://stackoverflow.com/questions/28234960/java-generics-and-enum-loss-of-template-parameters ). It *forces* you to implement the method for each enum, and then you can just iterate them with `values()`. — EpicPandaForce, Mar 12 '15 at 11:45

score 2 · Answer 2 · answered Mar 12 '15 at 11:38

You could put the process method as an abstract method in TaskType, and then override it in every task in the enum. What would probably be a better idea is if you create an interface, something like:

public interface Task {
    void process();
}

Then you either let your enum implement this interface. Or, probably better, you create concrete classes implementing this interface. One class for each of your task types.

score 1 · Answer 3 · answered Mar 12 '15 at 11:46

AFAIK you can't do it "automatically".

To minimize the risk of forgetting to add an if/case for new value you could have one "service" class for each enum value and a factory which provides a specific service for enum value.

E.g. instead of:

void methodA(TaskType type) {
   doSth();
   switch(type) {
      case TYPE_ONE:
        foo1(); 
        break;
      case TYPE_TWO:
        foo2();
        break;
      ...
   }
}
void methodB(TaskType type) {
   doSthElse();
   switch(type) {
      case TYPE_ONE:
        bar1(); 
        break;
      case TYPE_TWO:
        bar2();
        break;
      ...
   }
}

do:

interface Service {
   foo();
   bar();
}
class ServiceFactory {
   Service getInstance(TaskType type) {
      switch(type) {
         case TYPE_ONE:
            return new TypeOneService();
         case TYPE_TWO:
            return new TypeTwoService();
         default:
            throw new IllegalArgumentException("Unsupported TaskType: " + type);
      }
   }
}

And then the methods above can be rewritten as follows:

void methodX(TaskType type) {
   doSth();
   ServiceFactory.getInstance(type).foo();
}

This way you have only one point where you have to add handling of new enum value.

Stephen C · Accepted Answer · 2015-03-12T12:15:14.377

1

I think you are saying that you are wanting the compiler to tell you that all of the enum's values are considered.

Unfortunately, Java doesn't support that.

You might think that you could write something like this:

public int method(TaskType t) {
    switch (t) {
    case TYPE_ONE: return 1;
    case TYPE_TWO: return 2;
    case TYPE_THREE: return 3;
    }
    // not reachable ... no return required
}

... and rely on the compiler to tell you if you left out one of the enum values in the switch cases.

Unfortunately, it doesn't work!! The above is a compilation error anyway. According to the JLS reachability rules, the switch statement needs a default: arm for that method to be valid. (Or you can add a return at the end ...)

There is a good reason for this oddity. The JLS binary compatibility rules say that adding a new value to an enum is a binary compatible change. That means that any code with switch statement that switches on an enum needs to still remain valid (executable) code after the addition of enum values. If method was valid to start with, it can't become invalid (because there is a return path with no return statement) after the binary compatible change.

In fact, this is how I would write the code above:

public int method(TaskType t) {
    switch (t) {
    case TYPE_ONE: return 1;
    case TYPE_TWO: return 2;
    case TYPE_THREE: return 3;
    default:
       throw new AssertionError("TaskType " + t + " not implemented");
    }
    // not reachable ... no return required
}

This doesn't pretend to be compile-time safe, but it is fail-fast, and it doesn't involve bad OO design.

edited Mar 12 '15 at 12:15

answered Mar 12 '15 at 11:48

Stephen C

698,415
94
811
1,216

This is why they should make the enum-specific logic be part of the enum itself as per OldCurmudgeon's solution, this way you don't need to branch based on the `TaskType`. – EpicPandaForce Mar 12 '15 at 11:52
1

@EpicPandaForce - That solution doesn't scale. And besides it is bad design ... it violates the principle of separation of concerns. – Stephen C Mar 12 '15 at 11:58
It's better than having to modify the code whereever you need to iterate your enums every time you add a new enum. You can even make a separate class and return that from your enum which would "handle" the enum that the method is called on. It can work. – EpicPandaForce Mar 12 '15 at 12:00
It doesn't avoid that. You still have to modify the code ... in a different place. – Stephen C Mar 12 '15 at 12:02
I really just think that having to modify random `if-else` branch in the code is more of a ticking time bomb, because someone will eventually forget to add the new enum to one of them. – EpicPandaForce Mar 12 '15 at 12:03
Exactly what I meant! The question came up because I thought the Interface solution would break the Separation of Concerns principle in my case. Thanks for the explanation. – HexFlex Mar 12 '15 at 13:30

score 0 · Answer 5 · answered Mar 12 '15 at 11:40

HashMap<String, Integer> hm=new HashMap<String, Integer>();

...

if(taskType == TaskType.TYPE_ONE) {
    typeOneProcessing();
    hm.put(TaskType.TYPE_ONE, 1)
} else if(taskType == TaskType.TYPE_TWO) {
    typeTwoProcessing();
    hm.put(TaskType.TYPE_TWO, 1)
} else if(taskType == TaskType.TYPE_THREE) {
    typeThreeProcessing();
    hm.put(TaskType.TYPE_THREE, 1)
}

...

for (TaskType t : TaskType.values()) {
  if(hm.get(t)!=1)
     // Trigger the alarm
}

You can even count the times the element was count if you need it

score 0 · Answer 6 · answered Mar 12 '15 at 11:43

0

You can do swich case on the enum, and fail if the default is hit:

switch(taskType ){
  case TYPE_ONE: ... break;
  case TYPE_TWO: ... break;
  case TYPE_THREE: ... break;
  default: 
     throw new IllegalStateException("Unsupported task type:"+taskType);
  }

answered Mar 12 '15 at 11:43

Chriss

5,157
7
41
75

1

That won't detect the problem at compile-time. It will detect it at runtime. – JB Nizet Mar 12 '15 at 11:44

Ensure every enum value is used

6 Answers6