Should I be trying to create a reversible enum in Java or is there a better way?

Question

I seem to have faced this problem many times and I wanted to ask the community whether I am just barking up the wrong tree. Basically my question can be distilled down to this: if I have an enum (in Java) for which the values are important, should I be using an enum at all or is there a better way, and if I do use an enum then what is the best way to reverse the lookup?

Here's an example. Suppose I want to create a bean representing a specific month and year. I might create something like the following:

public interface MonthAndYear {
    Month getMonth();
    void setMonth(Month month);
    int getYear();
    void setYear(int year);
}

Here I'm storing my month as a separate class called Month, so that it is type-safe. If I just put int, then anyone could pass in 13 or 5,643 or -100 as a number, and there would be no way to check for that at compile-time. I'm restricting them to put a month which I'll implement as an enum:

public enum Month {
    JANUARY,
    FEBRUARY,
    MARCH,
    APRIL,
    MAY,
    JUNE,
    JULY,
    AUGUST,
    SEPTEMBER,
    OCTOBER,
    NOVEMBER,
    DECEMBER;
}

Now suppose that I have some backend database I want to write to, which only accepts the integer form. Well the standard way to do this seems to be:

public enum Month {
    JANUARY(1),
    FEBRUARY(2),
    MARCH(3),
    APRIL(4),
    MAY(5),
    JUNE(6),
    JULY(7),
    AUGUST(8),
    SEPTEMBER(9),
    OCTOBER(10),
    NOVEMBER(11),
    DECEMBER(12);

    private int monthNum;
    public Month(int monthNum) {
        this.monthNum = monthNum;
    }

    public getMonthNum() {
        return monthNum;
    }
}

Fairly straightforward, but what happens if I want to read these values from the database as well as writing them? I could just implement a static function using a case statement within the enum that takes an int and returns the respective Month object. But this means if I changed anything, then I would have to change this function as well as the constructor arguments - change in two places. So here's what I've been doing. First off I created a reversible map class as follows:

public class ReversibleHashMap<K,V> extends java.util.HashMap<K,V> {
    private java.util.HashMap<V,K> reverseMap;

    public ReversibleHashMap() {
        super();
        reverseMap = new java.util.HashMap<V,K>();
    }

    @Override
    public V put(K k, V v) {
        reverseMap.put(v, k);
        return super.put(k,v);
    }

    public K reverseGet(V v) {
        return reverseMap.get(v);
    }
}

Then I implemented this within my enum instead of the constructor method:

public enum Month {
    JANUARY,
    FEBRUARY,
    MARCH,
    APRIL,
    MAY,
    JUNE,
    JULY,
    AUGUST,
    SEPTEMBER,
    OCTOBER,
    NOVEMBER,
    DECEMBER;

    private static ReversibleHashMap<java.lang.Integer,Month> monthNumMap;

    static {
        monthNumMap = new ReversibleHashMap<java.lang.Integer,Month>();
        monthNumMap.put(new java.lang.Integer(1),JANUARY);
        monthNumMap.put(new java.lang.Integer(2),FEBRUARY);
        monthNumMap.put(new java.lang.Integer(3),MARCH);
        monthNumMap.put(new java.lang.Integer(4),APRIL);
        monthNumMap.put(new java.lang.Integer(5),MAY);
        monthNumMap.put(new java.lang.Integer(6),JUNE);
        monthNumMap.put(new java.lang.Integer(7),JULY);
        monthNumMap.put(new java.lang.Integer(8),AUGUST);
        monthNumMap.put(new java.lang.Integer(9),SEPTEMBER);
        monthNumMap.put(new java.lang.Integer(10),OCTOBER);
        monthNumMap.put(new java.lang.Integer(11),NOVEMBER);
        monthNumMap.put(new java.lang.Integer(12),DECEMBER);
    }

    public int getMonthNum() {
        return monthNumMap.reverseGet(this);
    }

    public static Month fromInt(int monthNum) {
        return monthNumMap.get(new java.lang.Integer(monthNum));
    }
}

Now this does everything I want it to, but it still looks wrong. People have suggested to me "if the enumeration has a meaningful internal value, you should be using constants instead". However, I don't know how that approach would give me the type-safety I am looking for. The way I've developed does seem overly complicated though. Is there some standard way to do this kind of thing?

PS: I know that the likelihood of the government adding a new month is...fairly unlikely, but think of the bigger picture - there are plenty of uses for enums.

Answer 1

There's a way easier way of doing this. Every enum has an ordinal() method return it's number (starting from zero).

public enum Month {
  JANUARY,
  FEBRUARY,
  MARCH,
  APRIL,
  MAY,
  JUNE,
  JULY,
  AUGUST,
  SEPTEMBER,
  OCTOBER,
  NOVEMBER,
  DECEMBER;

  public Month previous() {
    int prev = ordinal() - 1;
    if (prev < 0) {
      prev += values().length;
    }
    return values()[prev];
  }

  public Month next() {
    int next = ordinal() + 1;
    if (next >= values().length) {
      next = 0;
    }
    return values()[next];
  }
}

As for how to store this in a database, it depends on what persistence framework (if any) you're using. JPA/Hibernate have the option of mapping enum values by either number (ordinal) or name. Months are something you can probably take as non-changing so just use the ordinal. To get a specific value:

Month.values()[ordinalNumber];

Answer 2

This is a very common pattern, and it's fine for enums... but it can be implemented more simply. There's no need for a "reversible map" - the version which takes the month number in the constructor is better for going from Month to int. But going the other way isn't too hard either:

public enum Month {
    JANUARY(1),
    FEBRUARY(2),
    MARCH(3),
    APRIL(4),
    MAY(5),
    JUNE(6),
    JULY(7),
    AUGUST(8),
    SEPTEMBER(9),
    OCTOBER(10),
    NOVEMBER(11),
    DECEMBER(12);

    private static final Map<Integer, Month> numberToMonthMap;

    private final int monthNum;

    static {
        numberToMonthMap = new HashMap<Integer, Month>();
        for (Month month : EnumSet.allOf(Month.class)) {
            numberToMonthMap.put(month.getMonthNum(), month);
        }
    }

    private Month(int monthNum) {
        this.monthNum = monthNum;
    }

    public int getMonthNum() {
        return monthNum;
    }

    public static Month fromMonthNum(int value) {
        Month ret = numberToMonthMap.get(value);
        if (ret == null) {
            throw new IllegalArgumentException(); // Or just return null
        }
        return ret;
    }
}

In the specific case of numbers which you know will go from 1 to N, you could simply use an array - either taking Month.values()[value - 1] or caching the return value of Month.values() to prevent creating a new array on every call. (And as cletus says, getMonthNum could just return ordinal() + 1.)

However, it's worth being aware of the above pattern in the more general case where the values may be out of order, or sparsely distributed.

It's important to note that the static initializer is executed after all the enum values are created. It would be nice to just write

numberToMonthMap.put(monthNum, this);

in the constructor and add a static variable initializer for numberToMonthMap, but that doesn't work - you'd get a NullReferenceException immediately, because you'd be trying to put the value into a map which didn't exist yet :(

Answer 3

I'm probably far behind the pack in an answer here but I tend to implement it a little bit simpler. Don't forget that 'Enum' has a values() method.

public static Month parse(int num)
{
  for(Month value : values())
  {
    if (value.monthNum == num)
    {
      return value;
    }
  }
  return null; //or throw exception if you're of that mindset
}

Answer 4

You shouldn't use ordinal() for this kind of thing, for the sample with months it would work (because it will not be extended) but one of the good things with enums in java is that they are designed to be possible to extend without breaking things. If you start relying on ordinal() things will break if you add some value in the middle.

I would do it like Jon Skeet suggests (he wrote it while I was writing this) but for cases where the internal number representation is in a well defined range of say 0 to 20 (or something) I would probably not use a HashMap and introduce autoboxing of the int but rather use an ordinary array (like Month[12]) but both are fine (Jon later changed his post to include this suggestion).

Edit: For the few enums where there is a natural order (like sorted months) ordinal() is probably safe to use. The problems you risk running into if you persist it will appear for things where someone might change the order of the enum. Like if: the "enum { MALE, FEMALE }" becomes an "enum {UNKNOWN, FEMALE, MALE}" when someone extends the program in the future not knowing that you rely on ordinal.

Giving Jon a +1 for writing the same I was just writing.