7

At work I bumped into a problem to see how enums are growing and storing business logics. As they grew, the constructors grew a lot. At one point I found out instead of putting in let's say the tenth boolean parameter (and which already had various other parameters than boolean) into the constructor, and expand all the initializations with an extra parameter, I will make a new - yet not really used - construction for purpose of code readability.

Here is an example, so instead of:

public enum A {
  A1(true, true, false),
  A2(true, false, true),
  A3(false, false, false);

private A(boolean firstParam, boolean secondParam, boolean thirdParam){
    ... usual constructor ...
}

... getters

So instead I did the following:

public enum A {
  A1,
  A2,
  A3;

... no need for special constructor

public boolean isFirstParam(){
  switch(this){
    case A1:
    case A2:
      return true;
    default:
      return false;
  }
}

public boolean isSecondParam(){
  switch(this){
    case A1:
      return true;
    default:
      return false;
  }
}

public boolean isThirdParam(){
  switch(this){
    case A2:
      return true;
    default:
      return false;
  }
}

So far whoever used it likes it. But some collegues who were unaware of it had bad reactions:

  • Sonar was unhappy to see the switch(this) statement.
  • They didn't like the concept itself.

My question is: is there a real reason why I shouldn't use it? It is way more maintainable when you have complex enums. I do believe I didn't use anything special, just switching over enum, just in a special place. Am I right?

  • 1
    What alternative were they suggesting? I switch over enums a lot, because I know of no better construct. Perhaps they know one? – Kilian Foth Nov 18 '15 at 14:37
  • @KilianFoth They didn't really suggest alternative but the "good old way" to make the constructor and expand it (infinitely) – CsBalazsHungary Nov 18 '15 at 14:39
  • 1
    That is definitely not a better alternative - the values for a lot of fields are specified in the form a literals far away from their definitions, which makes the code almost impossible to understand. In that case, my answer is "you should definitely use switch". – Kilian Foth Nov 18 '15 at 14:41
13

The other alternative is overriding methods per instance:

public enum A {
  A1{
    @Override public boolean isFirstParam(){ return true; }
    @Override public boolean isSecondParam(){ return true; }
    @Override public boolean isThirdParam(){ return false; }
  },
  A2{
    @Override public boolean isFirstParam(){ return true; }
    @Override public boolean isSecondParam(){ return false; }
    @Override public boolean isThirdParam(){ return false; }
  },
  A3{
    @Override public boolean isFirstParam(){ return false; }
    @Override public boolean isSecondParam(){ return false; }
    @Override public boolean isThirdParam(){ return false; }
  };

  public abstract boolean isFirstParam();
  public abstract boolean isSecondParam();
  public abstract boolean isThirdParam();
}

https://docs.oracle.com/javase/8/docs/technotes/guides/language/enums.html has this to say:

The idea of adding behavior to enum constants can be taken one step further. You can give each enum constant a different behavior for some method. One way to do this by switching on the enumeration constant. Here is an example with an enum whose constants represent the four basic arithmetic operations, and whose eval method performs the operation:

public enum Operation {
    PLUS, MINUS, TIMES, DIVIDE;

    // Do arithmetic op represented by this constant
    double eval(double x, double y){
        switch(this) {
            case PLUS:   return x + y;
            case MINUS:  return x - y;
            case TIMES:  return x * y;
            case DIVIDE: return x / y;
        }
        throw new AssertionError("Unknown op: " + this);
    }
}

This works fine, but it will not compile without the throw statement, which is not terribly pretty. Worse, you must remember to add a new case to the switch statement each time you add a new constant to Operation. If you forget, the eval method with fail, executing the aforementioned throw statement

There is another way give each enum constant a different behavior for some method that avoids these problems. You can declare the method abstract in the enum type and override it with a concrete method in each constant. Such methods are known as constant-specific methods. Here is the previous example redone using this technique:

public enum Operation {
  PLUS   { double eval(double x, double y) { return x + y; } },
  MINUS  { double eval(double x, double y) { return x - y; } },
  TIMES  { double eval(double x, double y) { return x * y; } },
  DIVIDE { double eval(double x, double y) { return x / y; } };

  // Do arithmetic op represented by this constant
  abstract double eval(double x, double y);
}

Personally I don't mind throwing an exception and explicitly handling all posibilities, except if the switch would be located in the enum itself. The IDE gives me a warning (I can even configure it to turn it into an error so it won't compile) when you don't handle all cases. If you keep your projects warning free, which I try to do, you will immidiently know which files need their switch statements updated.

  • Good answer, I am gladly accepting it. One thing I will use to improve the concept: throwing an error is even better so the code can return true and false, and if somebody forgot to put a new enum, it will throw an exception. Thanks! – CsBalazsHungary Nov 19 '15 at 7:50
  • @CsBalazsHungary Oh, I like not providing a default and throwing runtime exceptions too! I do that with almost every switch statement I write these days. Fail-Fast and all that. Which means that if we can fail with a compile time error instead of a runtime error, by not implementing an abstract method for example, then I prefer that aproach. – Darsstar Nov 19 '15 at 14:40
-1

I know this is old and answered already, but another option would be the builder pattern.

public enum A {
  A1(new EnumBuilder().set1(true).set2(true).set3(false)),
  A2(new EnumBuilder().set1(true).set2(false).set3(true)),
  A3(new EnumBuilder().set1(false).set2(false).set3(false));

private A(EnumBuilder builder){
    param1 = builder.param1;
    // etc.
}
...
}

class EnumBuilder {
    private boolean param1, param2, param3;

    EnumBuilder set1(boolean value){
        param1 = value;
        return this;
    }

    // etc.
}
  • Whoever downvoted, would you care to explain the problem? – nasch May 26 '18 at 22:06

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.