69

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.

Community
  • 1
  • 1
wchargin
  • 15,589
  • 12
  • 71
  • 110

7 Answers7

104

Try this:

public enum A {
    X, Y, Z;
    
    private static final 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.

Alexander Ivanchenko
  • 25,667
  • 5
  • 22
  • 46
Jim Garrison
  • 85,615
  • 20
  • 155
  • 190
  • 2
    Ah! I wasn't aware of `ordinal`; I looked for `indexOf`. Would it be better to declare `A[] values = values()` to avoid cloning twice? – wchargin Jun 09 '13 at 04:00
  • I'd be surprised if there was any cloning involved since enum values are singletons. You could always look at the generated code. – Jim Garrison Jun 09 '13 at 04:01
  • 1
    The `values` method returns `(A[]) ($VALUES.clone())` where `private static final A[] $VALUES = new A[] {X,Y,Z}`. See [this answer](http://stackoverflow.com/a/1163121/732016). – wchargin Jun 09 '13 at 04:01
  • 4
    Singleton doesn't matter when you could do `A.values()[0] = null` or `A.values()[0] = A.Z` and mess everything else up. Just because the array is final doesn't mean its contents are. – wchargin Jun 09 '13 at 04:02
  • @WChargin You could even make the `values` field static, since it'll be the same for all of the enum values. – yshavit Jun 09 '13 at 04:05
  • 1
    I just disassembled the code and find that it DOES clone, for the reasons pointed out by others. So I'd make a copy once and use that. – Jim Garrison Jun 09 '13 at 04:07
  • BTW, I compiled the code with Eclipse and disassembled with javap. It doesn't clone but `new` and then `System.arraycopy`. – johnchen902 Jun 09 '13 at 04:10
  • I'm hestitant to declare `private static final A[] vals` when you could `A.class.getField("vals").setAccessible(true)` and go to town... as far as I know reflection is impotent inside a method, assuming the classloader/VM hasn't been compromised. – wchargin Jun 09 '13 at 04:10
  • @johnchen902 in any case it's re-allocating memory. The method isn't really important (although `arraycopy` may be a bit faster). – wchargin Jun 09 '13 at 04:12
  • Well, the question is how paranoid you want to be... what's your objective? – Jim Garrison Jun 09 '13 at 04:13
  • @WChargin If you're worried about that, then a non-static wouldn't help you either. Someone could do the same and go to town by passing `E.A` (or any of the other enum values) instead of `null` to the first argument of the `Field` setters. If you're worried about reflection in that way, you need to take the performance "hit" (which I doubt would be in any way significant, unless this is in a very tight loop) of the clone each time. – yshavit Jun 09 '13 at 04:14
  • If someone has access to the machine running your Java code, security is compromised any way you slice it; they can just inject a javaagent and rewrite your bytecode to whatever they want. If they don't have direct access to it, but you allow arbitrary plugins, then you definitely need some sort of security manager anyway. Plus, I would question the choice to load untrusted third-party plugins onto a server-side app. So basically, I wouldn't worry too much about security from a reflection perspective, cause you basically don't have any. :) – yshavit Jun 09 '13 at 04:22
  • 1
    If you have an instance of "A" called "a" such as: `A a`. Then assign it like this: `a = a.next();` The call `a.next();` will not increment by itself. See Bohemian's answer in http://stackoverflow.com/questions/17664445/java-operator-for-enum – Joe Jan 28 '16 at 19:28
  • Just for fun, implemented this functionality [here](https://stackoverflow.com/a/74339419/17949945) using a `TreeSet`. – Alexander Ivanchenko Nov 06 '22 at 20:30
7

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.
Tom
  • 16,842
  • 17
  • 45
  • 54
GhostCat
  • 137,827
  • 25
  • 176
  • 248
  • Hi, GhostCat. I suppose that I hadn't stated this explicitly in the question, but I was hoping for an automatically extensible (i.e., not "brute force") solution, under the assumption that the default ordering is a logical one. I don't really think that this answers the question, but your point that the default ordering may not be desirable is indeed valuable. – wchargin Jan 03 '17 at 13:11
  • 1
    You are welcome; I just came across some other question and thought: especially that Optional part would be worth its own answer. – GhostCat Jan 03 '17 at 13:12
  • Yeah. I love what `Optional` could be and have given it a fair shot in larger projects; it's too bad that it fails to integrate well into the rest of the language. – wchargin Jan 03 '17 at 13:13
2
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];
    }
}
kruk
  • 75
  • 1
  • 4
1

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.

1

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);
  }
}
Zoltan
  • 2,928
  • 11
  • 25
1

TreeSet-based implementation

We can implement an enum capable of retrieving next and previous members via instance methods next() and previous() by using TreeSet, which maintains a Red-black tree under the hood, and offers methods for traversing the tree like higher() and lower().

The implementation below would in a way similar to a circular list, retrieving the fist enum-constant when next() is invoked on the very last constant, and vice versa returning the last when previous() is called on the very first constant.

Instead of hard-coding the fist and the last enum-members inside the next() and previous() when we hit edge-cases, we can use methods first() and last(). By doing so, we're eliminating the possibility of introducing a bug, if someone would decide to add a few more constants or reorder them.

public enum Enum {
    A, B, C, D, E, F, G;
    
    private static final NavigableSet<Enum> set =
        new TreeSet<>(EnumSet.allOf(Enum.class)); // EnumSet.allOf() generates a set of enum-constants of the specified type
    
    public Enum next() {
        return Objects.requireNonNullElseGet(
            set.higher(this), set::first
        );
    }
    
    public Enum previous() {
        return Objects.requireNonNullElseGet(
            set.lower(this), set::last
        );
    }
}

Note: both higher() and lower() would return null if the requested element doesn't exist. To dial with the edge-cases I've used Java 11 utility method Objects.requireNonNullElseGet() which expects a nullable value and a Supplier that would be used only if provided value is null (reminder: Java 8 functions are lazy).

main()

public static void main(String[] args) {

    EnumSet.allOf(Enum.class).forEach(e ->
        System.out.println(e + " -> " + " next: " + e.next() + " prev: " + e.previous())
    );
}

Output:

A ->  next: B prev: G
B ->  next: C prev: A
C ->  next: D prev: B
D ->  next: E prev: C
E ->  next: F prev: D
F ->  next: G prev: E
G ->  next: A prev: F

Performance Note

All the methods of the TreeSet used above have logarithmic time complexity O(log n), which in most of the real life scenarios would result in only a couple of steps through the tree, since the waste majority of enums have fewer than ten constants. Therefore, we can call it acceptable, but it can't beat constant time the performance of the straightforward solution provided in the answer by Jim Garrison.

That said, the code above is meant to serve educational purposes by illustrating how a TreeSet can be utilized.

Alexander Ivanchenko
  • 25,667
  • 5
  • 22
  • 46
0

Same approach as @Zoltan but without 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() {
      // required for Mercury
   }

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

   public Planet prev() {
      return this == MERCURY ? this : prevPlanet;
   }

   public Planet next() {
      return this == NEPTUNE ? this : nextPlanet;
   }
}
Elie Nehmé
  • 229
  • 2
  • 5