Logic inside an enum

My colleagues and I were having a discussion regarding logic in enums. My personal preference is to not have any sort of logic in Java enums (although Java provides the ability to do that). The discussion in this cased centered around having a convenience method inside the enum that returned a map:

public enum PackageType {
  Letter("01", "Letter"),
  ..
  ..
  Tube("02", "Packaging Tube");

  private String packageCode;
  private String packageDescription;

  ..
  ..

  public static Map<String, String> toMap() {
     Map<String, String> map = new LinkedHashMap<String, String>();
     for(PackageType packageType : PackageType.开发者_运维问答values()) {
         map.put(packageType.getPackageCode(), packageType.getPackageDescription());
     }
     return map;
  }
}

My personal preference is to pull this out into a service. The argument for having the method inside the enum centered around convenience. The idea was that you don't have to go to a service to get it, but can query the enum directly.

My argument centered around separation of concern and abstracting any kind of logic out to a service. I didn't think "convenience" was a strong argument to put this method inside an enum.

From a best-practices perspective, which one is better? Or does it simply come down to a matter of personal preference and code style?

Well, I've done this before but that certainly doesn't mean it's the 'best' thing to do.

From my perspective, though, I would prefer to have that logic on the enum, for the same reason you wouldn't move a 'toString' method out to a service. The logic only concerns the enum itself, and its own representation.

I think it would be misleading to move such a method out to a service - by placing it on the enum you are being up front about the fact that the enum has a 'toMap' method. Someone who didn't know about the service and was just looking at the enum may not know that.

It also helps with auto completion in IDE's - I can hit the '.' key and instantly see methods provided by the object.

I think it probably comes down to personal preference and whether or not you think the logic might change in the future.

The best use case for enum logic (since it's pretty static) is for things that aren't going to change. The logic in java.util.concurrent.TimeUnit is a pretty good example of this: the conversion factors between time units are well-defined and won't ever change, so it's a good candidate for static logic to embedded within the enum.

I cannot see any persuasive reason why it is "good practice" ... or "bad practice" ... to do what you are suggesting.

I'm inclined to go for the convenience argument. But frankly, this is not the kind of thing that it is productive to spend man-days debating ... IMO.

I have tried logic in enums a few times, but the only thing I am ever really happy about is something like:

// not compiled, so might have errors...
public enum Foo
{
   A,
   B;

   // not complete, doesn't properly handle invalid cases...
   public static Foo fromString(final String str)
   {
       final String strLower;
       final Foo    val;

       strLower = str.toLowerCase();

       if(strLower.equals("a")
       {
           val = A;
       }
       else if(strLower.equals("b")
       {
           val = B;
       }
       else
       {
           throw new IllegalArgumentException(/*...*/);
       }

       return (val);
   }
}

Generally once you start adding instance variables/methods you should probably be doing something with a proper class.

I'd start with putting it in the enum. If there will be only one toMap() method in the system, having an extra class is surely unnecessary and will be annoying.

If I had bunch of enums from which I want to create such map, or maybe anticipate such situation, THEN I'll create a static utility class, and a generified utility method to do that.

My personal opinion is, practicality tramps theory.

EDIT: Oh wait, this would be difficult to provide a generic utility method for.. In that case, if the method is going to be that enum specific, I'd definitely go with putting it in the enum. If I were the maintenance programmer, I'd be very dissatisfied if you would have an extra class that I have to look for.

Like anything it would depend on the usage, there are general rules but practicality should always win the argument. What is the map used for? Would you ever need to restrict the contents of the map, e.g. if the map is used to populate a combo box, is there ever the need to remove some choices, say if some carriers only handled certain package types a strategy could be used to populate a map, that would be best done outside of the enum.

Despite that, my preference would be to have a create the map elsewhere, that extra level of indirection is never a hindrance to flexibility and could save a lot of grief and adds little overhead now. And you can also code it so you can get similar functionality with other enums.