Self referential enum with immutable parameters

Question

Consider the following sscce

public enum Flippable 
  A (Z), B (Y), Y (B), Z (A);

  private final Flippable opposite;

  private Flippable(Flippable opposite) {
    this.opposite = opposite;
  }

  public Flippable flip() {
    return opposite;
  }
}

This doesn't compile, because Z and Y haven't been declared to be allowed to be arguments of A and B's constructor.

Potential solution 1: Hardcoded Methods

public enum Flippable {
  A {
    public Flippable flip() { return Z; }
  }, B {
    public Flippable flip() { return Y; }
  }, Y {
    public Flippable flip() { return B; }
  }, Z {
    public Flippable flip() { return A; }
  };
  public abstract Flippable flip();
}

While functional, this seems stylistically quite gross. Though I can't put a finger on why this would be a real problem.

Potential solution 2: static loading

public enum Flippable {
  A, B, Y, Z;

  private Flippable opposite;

  static {
    for(Flippable f : Flippable.values()) {
      switch(f) {
      case A:
        f.opposite = Z;
        break;
      case B:
        f.opposite = Y;
        break;
      case Y:
        f.opposite = B;
        break;
      case Z:
        f.opposite = A;
        break;
      }
    }
  }

  public Flippable flip() {
    return opposite;
  }
}

This is even more gross than the first solution, as the field is no longer final, and is vulnerable to reflection. Ultimately that is an obscure worry, but suggests a bad code smell.

Is there a way to do this that is essentially the same as the first example, but compiles properly?

why are you enumerating the Flippables? Why not just assign each directly in turn, since you have to do this in each `case:` expression anyway! — robert, Oct 17 '13 at 14:52

score 18 · Accepted Answer · edited Oct 17 '13 at 15:00

18

Again perhaps not as pretty as you were looking for ...

public enum Flippable {
    A, B, Z, Y;

    static {
        A.opposite = Z;
        B.opposite = Y;
        Y.opposite = B;
        Z.opposite = A;
    }

    public Flippable flip() {
        return opposite;
    }

    private Flippable opposite;

    public static void main(String[] args) {         
        for(Flippable f : Flippable.values()) {
            System.out.println(f + " flips to " + f.flip());
        }
    }
}

edited Oct 17 '13 at 15:00

durron597

31,968
17
99
158

answered Oct 17 '13 at 14:39

robert

4,612
2
29
39

I prefer this solution out of all presented so far (including mine), but I feel as if there should be some sort of guarantee that `A.flip().flip()` should always return `A`. This solution (as well as mine) is error-prone to typos. – splungebob Oct 17 '13 at 15:12
I don't think there's any way around the circular referencing problem – robert Oct 17 '13 at 15:18
1

@Admit This answer is faster than yours. [This is a link to my benchmark, results are commented at the top](http://pastebin.com/pnr5bRVw) – durron597 Oct 17 '13 at 15:35

Admit · Answer 2 · 2013-10-17T15:05:53.420

4

As you can see it's not possible due to enum constants are static and you could not initialize A until Z is not initialized.

So this trick should work:

public enum Flippable { 
  A ("Z"), B ("Y"), Y ("B"), Z ("A");

  private final String opposite;

  private Flippable(String opposite) {
    this.opposite = opposite;
  }

  public Flippable flip() {
    return valueOf(opposite);
  }
}

edited Oct 17 '13 at 15:05

answered Oct 17 '13 at 14:36

Admit

4,897
4
18
26

you might also put the `valueOf` into your constructor so you're storing the actual value rather than the String ... – robert Oct 17 '13 at 14:49
@robert No he can't, it doesn't run. I don't like this answer as it's slower (string parsing every time)... unless Hotspot is smart enough to fix the problem. – durron597 Oct 17 '13 at 14:54
If its in the constructor it should only parse once at class-load - but yeah, I just checked and it compiles fine but doesn't run (: – robert Oct 17 '13 at 14:56
1

If you look to `Enum.valueOf()` it internally uses `Map` and perform search by key there, so it's quite fast. In any case it's up to your case, what to choose. You did not specify what is your criteria: speed, memory consumption etc. This answer definitely is applicable to your question: Is there a way to do this that is essentially the same as the first example, but compiles properly? – Admit Oct 17 '13 at 15:14
Another fair point. Okay I've upvoted you; actually given the speed of `Enum.valueOf` this solves the immutability problem too, so this actually is probably the best answer. – durron597 Oct 17 '13 at 15:23
I like this answer the best. Concise, immutable and performant. – Jeshurun Mar 26 '18 at 20:27

splungebob · Answer 3 · 2013-10-17T17:35:52.983

2

Just map the opposites:

import java.util.*;

public enum Flippable 
{
  A, B, Y, Z;

  private static final Map<Flippable, Flippable> opposites;

  static
  {
    opposites = new EnumMap<Flippable, Flippable>(Flippable.class);
    opposites.put(A, Z);
    opposites.put(B, Y);
    opposites.put(Y, B);
    opposites.put(Z, A);

    // integrity check:
    for (Flippable f : Flippable.values())
    {
      if (f.flip().flip() != f)
      {
        throw new IllegalStateException("Flippable " + f + " inconsistent.");
      }
    }
  }

  public Flippable flip()
  {
    return opposites.get(this);
  }

  public static void main(String[] args)
  {
    System.out.println(Flippable.A.flip());
  }
}

EDIT: switched to EnumMap

edited Oct 17 '13 at 17:35

answered Oct 17 '13 at 14:35

splungebob

5,357
2
22
45

This is no better than solution 2, in fact it's worse because it has higher memory consumption – durron597 Oct 17 '13 at 14:36
I guess I'm all for readability. Solution 2 (like most switches) looks ugly to me. So I guess your claim of "worse" is highly subjective. IMHO, memory consumption doesn't seem like a legitimate argument when considering maps vs. logically similar switches. – splungebob Oct 17 '13 at 14:43
The switch happens statically, and only once. – durron597 Oct 17 '13 at 14:49
1

yeah there's no need for the HashMap here, you can assign to the member directly – robert Oct 17 '13 at 14:51
4

I'd agree that there's no advantage to using a map over robert's answer. But if someone told you you had to use a map, an `EnumMap` would be better than a `HashMap`. – ajb Oct 17 '13 at 14:54
@ajb +1 about the `EnumMap`. – splungebob Oct 17 '13 at 14:59
@splungebob When you switch to an `EnumMap`, this runs almost as fast as @robert's (under 10% slower), but it has the advantage that, if using Guava 14+, you can wrap the map in an `ImmutableEnumMap` that helps protect against reflection attacks at almost no additional cost. – durron597 Oct 17 '13 at 17:26
Another feature that could be added is that instead of adding to the map directly, you could have a `private static void assign(Flippable source, Flippable opposite)` that can do integrity checks before adding to the map, such as (a) prevent duplicate keys, (b) prevent duplicate opposites, or (c) ensure consistency in opposites. Also, see my addition to the static block that checks for flipping "integrity". – splungebob Oct 17 '13 at 17:38

Ed Price · Answer 4 · 2016-01-20T20:57:43.093

0

Nice question. Perhaps you will like this solution:

public class Test {
    public enum Flippable {
        A, B, Y, Z;

        private Flippable opposite;

        static {
            final Flippable[] a = Flippable.values();
            final int n = a.length;
            for (int i = 0; i < n; i++)
                a[i].opposite = a[n - i - 1];
        }

        public Flippable flip() {
            return opposite;
        }
    }

    public static void main(final String[] args) {
        for (final Flippable f: Flippable.values()) {
            System.out.println(f + " opposite: " + f.flip());
        }
    }
}

Result:

$ javac Test.java && java Test
A opposite: Z
B opposite: Y
Y opposite: B
Z opposite: A
$

If you want to keep the instance field "final" (which is certainly nice), you could index into the array at runtime:

public class Test {
    public enum Flippable {
        A(3), B(2), Y(1), Z(0);

        private final int opposite;
        private Flippable(final int opposite) {
            this.opposite = opposite;
        }

        public Flippable flip() {
            return values()[opposite];
        }
    }

    public static void main(final String[] args) {
        for (final Flippable f: Flippable.values()) {
            System.out.println(f + " opposite: " + f.flip());
        }
    }
}

which also works.

edited Jan 20 '16 at 20:57

answered Jan 20 '16 at 20:55

Ed Price

11
2

Interesting approach. Why do you think this is preferred over the other answers? – durron597 Jan 20 '16 at 20:57
Well, the first solution automatically computes the opposites instead of hard-coding them, which is arguably even cleaner than the desired (but not valid Java) original! OTOH, it loses immutability, which is a shame. – Ed Price Jan 20 '16 at 21:05
The second solution is basically identical to the desired original except with numbers instead of names to specify the opposites - and a bit of runtime cost for the array access, admittedly. So neither is perfect, unfortunately, but I think they both improve on other answers in some way... – Ed Price Jan 20 '16 at 21:07
1

I have had very bad experience with depending on the index value of an enum. Sooner or later you might accidentally swap the indices and [diamonds become black, when they should be red](http://codereview.stackexchange.com/questions/36916/weekend-challenge-poker-hand-evaluation#comment60836_36936). – Simon Forsberg Jan 20 '16 at 21:34
Good point Simon. The numbers are fragile... The hardcoded names are fragile too though. So that's a reason to prefer my first solution, which hardcodes neither :) – Ed Price Jan 21 '16 at 14:49

score 0 · Answer 5 · answered Mar 01 '17 at 13:05

As long as the field remains private to the enum, I am not sure whether the final status of it is truly of much concern.

That said, if an opposite is defined in the constructor (it doesn't have to be!), the entry takes said opposite into its own field while assigning itself to the field of the opposite. This should all be easily resolvable throujhgh finals.

fkurth · Answer 6 · 2017-05-19T07:07:33.673

I'm using this solution:

public enum Flippable {

    A, B, Y, Z;

    public Flippable flip() {
        switch (this) {
            case A:
                return Z;
            case B:
                return Y;
            case Y:
                return B;
            case Z:
                return A;
            default:
                return null;
        }
    }
}

Test:

public class FlippableTest {
    @Test
    public void flip() throws Exception {
        for (Flippable flippable : Flippable.values()) {
            assertNotNull( flippable.flip() ); // ensure all values are mapped.
        }
        assertSame( Flippable.A.flip() , Flippable.Z);
        assertSame( Flippable.B.flip() , Flippable.Y);
        assertSame( Flippable.Y.flip() , Flippable.B);
        assertSame( Flippable.Z.flip() , Flippable.A);
    }
}

This version is even shorter, but is limited in flexibility:

public enum Flippable {

    A, B, Y, Z;

    public Flippable flip() {
            return values()[ values().length - 1 - ordinal() ];
    }

}

I'm interested to see your first answer benchmarked against the others. — durron597, May 19 '17 at 07:31

Jeshurun · Answer 7 · 2018-03-26T20:06:54.963

0

The code below compiles fine, and meets all of the OP's requirements, but fails at runtime with Exception in thread "main" java.lang.ExceptionInInitializerError. It is left here to rot as an example of what not to do for all those who stumble upon it.

public enum Flippable {
A, B, Y, Z;

private final Flippable opposite;

private Flippable() {
    this.opposite = getOpposite(this);
    verifyIntegrity();
}

private final Flippable getOpposite(Flippable f) {
    switch (f) {
        case A: return Z;
        case B: return Y;
        case Y: return B;
        case Z: return A;
        default:
            throw new IllegalStateException("Flippable not found.");
    }
}

private void verifyIntegrity() {
    // integrity check:
    Arrays.stream(Flippable.values())
    .forEach(f -> {
        if(!f.flip().flip().equals(f)) {
            throw new IllegalStateException("Flippable " + f + " is inconsistent.");
        }
    });
}

public Flippable flip() {
    return opposite;
}

}

edited Mar 26 '18 at 20:06

answered Mar 26 '18 at 17:00

Jeshurun

22,940
6
79
92

Why are you building the map fresh every time? – durron597 Mar 26 '18 at 19:01
I don't have to. I could declare it `static final` and initialize it in a static block. But then the Map instance is sticking around for the duration of execution of your program, but never needed again after the initial constructor run. Initializing it every time is a trade off wherein you take an initial performance hit, but reduced memory thereafter as the Map isn't sticking around forever. – Jeshurun Mar 26 '18 at 19:47
actually it looks like you don't need the Map at all. You can get away with a switch as long as it is in a separate method? I've updated the answer. – Jeshurun Mar 26 '18 at 19:55
Interestingly enough, this compiles fine but throws an `Exception in thread "main" java.lang.ExceptionInInitializerError`. I'm going to leave this here as an example of what not to do. – Jeshurun Mar 26 '18 at 19:58

Self referential enum with immutable parameters

7 Answers7

Linked