What's the best way to implement `next` and `previous` on an enum type?

Suppose I have an enum:

enum E {
    A, B, C;
}

As shown in this answer by lucasmo, enum values are stored in a static array in the order that they are initialized, and you can later retrieve (a clone of) this array with E.values().

Now suppose I want to implement E#getNext and E#getPrevious such that all of the following expressions evaluate to true:

E.A.getNext() == E.B
E.B.getNext() == E.C
E.C.getNext() == E.A

E.A.getPrevious() == E.C
E.B.getPrevious() == E.A
E.C.getPrevious() == E.B

My current implementation for getNext is the following:

public E getNext() {
    E[] e = E.values();
    int i = 0;
    for (; e[i] != this; i++)
    	;
    i++;
    i %= e.length;
    return e[i];
}

and a similar method for getPrevious.

However, this code seems cumbersome at best (e.g., "empty" for loop, arguable abuse of a counter variable, and potentially erroneous at worst (thinking reflection, possibly).

What would be the best way to implement getNext and getPrevious methods for enum types in Java 7?

---

NOTE: I do not intend this question to be subjective. My request for the "best" implementation is shorthand for asking for the implementation that is the fastest, cleanest, and most maintainable.

Try this:

public static enum A { 
    X, Y, Z;
    private static A[] vals = values();
    public A next()
    {
        return vals[(this.ordinal()+1) % vals.length];
    }

Implementation of previous() is left as an exercise, but recall that in Java, the modulo a % b can return a negative number.

EDIT: As suggested, make a private static copy of the values() array to avoid array copying each time next() or previous() is called.

Alternatively, one can go somehow along the lines of the following idea:

public enum SomeEnum {
  A, B, C;

  public Optional<SomeEnum> next() {
    switch (this) {
      case A: return Optional.of(B);
      case B: return Optional.of(C);
      // any other case can NOT be mapped!
      default: return Optional.empty();
  }
}

Notes:

1. In contrast to the other answer, this way does some implicit mapping; instead of relying on ordinal(). Of course that means more code; but it also forces the author to consider what it means to add new constants or remove existing ones. When relying on ordinal, your implicit assumption is that the order is based on the order used for the enum constant declaration. So when somebody comes back 6 months later and has to add a new constant, he has to understand that the new constant Y needs X, Y, Z ... instead of just appending X, Z, Y!
2. There might be situations where it doesn't make any sense for the "last" enum constant to have the "first" as successor. Think of T-Shirt sizes for examples. XXL.next() is for sure not XS. For such situations, using Optional is the more appropriate answer.

public enum Three
{
    One, Two, Three;

    static 
    public final Three[] values = values();

    public Three prev() {
        return values[(ordinal() - 1  + values.length) % values.length];
    }

    public Three next() {
        return values[(ordinal() + 1) % values.length];
    }
}

Here's another take at the problem:

public enum Planet {
  MERCURY, VENUS, EARTH, MARS, JUPITER, SATURN, URANUS, NEPTUNE;

  private Planet prevPlanet = null;
  private Planet nextPlanet = null;

  static {
    for (int i = 1; i <= values.length; i++) {
      Planet current = values[i % values.length];
      current.prevPlanet = values[i - 1];
      current.nextPlanet = values[(i + 1) % values.length];
    }
  }

  public Planet prev() {
    return prevPlanet;
  }

  public Planet next() {
    return nextPlanet;
  }
}

With this approach, all calculations are done during static initialization and the actual methods directly return the result from a member variable.

However, I would argue that for this enum (and for most enums in general), wrapping around doesn't make sense, so I would rather do it this way:

import java.util.Optional;

public enum Planet {
  MERCURY, VENUS, EARTH, MARS, JUPITER, SATURN, URANUS, NEPTUNE;

  private Planet prevPlanet = null;
  private Planet nextPlanet = null;

  static {
    Planet[] values = Planet.values();
    for (int i = 1; i < values.length; i++) {
      values[i].prevPlanet = values[i - 1];
    }
    for (int i = 0; i < values.length - 1; i++) {
      values[i].nextPlanet = values[i + 1];
    }
  }

  public Optional<Planet> prev() {
    return Optional.ofNullable(prevPlanet);
  }

  public Optional<Planet> next() {
    return Optional.ofNullable(nextPlanet);
  }
}

Here, the first planet does not have a previous one and the last one does not have a next one. Optional is used to make it even more explicit that callers of the code need to be prepared that not every planet has a next/previous one. Whether you want to use Optional is up to you, the code works just as well with the getters of the first implementation, in which case a null would be returned directly instead of as an empty Optional.

Another thing to consider is that the desired ordering may not match the enumeration of the values. There could also be special values in the enum that do not fit in the ordering. Or you could just want to make the specification of the ordering explicit so that one can not accidentally break the logic by adding a new value to the enum out of order. Then you can do this:

import java.util.Optional;

public enum Planet {
  MERCURY, VENUS(MERCURY), EARTH(VENUS), MARS(EARTH), JUPITER(MARS),
    SATURN(JUPITER), URANUS(SATURN), NEPTUNE(URANUS);

  private Planet prevPlanet = null;
  private Planet nextPlanet = null;

  Planet() {}

  Planet(Planet prev) {
    this.prevPlanet = prev;
    prev.nextPlanet = this;
  }

  public Optional<Planet> prev() {
    return Optional.ofNullable(prevPlanet);
  }

  public Optional<Planet> next() {
    return Optional.ofNullable(nextPlanet);
  }
}

This doesn't allow for wraparounds, but you could use this as a low-impact way to check adjacency:

enum Phase {
    ONE, TWO, THREE;
    public final Phase previous;
    Phase() {
        previous = Data.last;
        Data.last = this
    }
    private static class Data {
        private static Phase last = null;
    }
}

class Example {
    Phase currentPhase = Phase.ONE;

    void advanceToPhase(Phase nextPhase) {
        if (nextPhase.previous == currentPhase)
            currentPhase = nextPhase;
    }
}

It has to use an auxiliary static class to store a variable for the static initializer, but it has the advantage of being extremely low-cost at startup.