Search code examples
javaenumsnullpointerexceptionjlscyclic-reference

What is the explanation of null values in a cyclic dependency in final private enum fields?


Consider these enum declarations:

enum Color {
    RED(Shape.CIRCLE),
    GREEN(Shape.TRIANGLE),
    BLUE(Shape.SQUARE);

    private final Shape shape;

    Color(Shape shape) {
        this.shape = shape;
    }

    Shape getShape() {
        return shape;
    }
}

enum Shape {
    CIRCLE(Color.RED),
    TRIANGLE(Color.GREEN),
    SQUARE(Color.BLUE);

    private final Color color;

    Shape(Color color) {
        this.color = color;
    }

    Color getColor() {
        return color;
    }
}

There are cyclic dependencies between the enum fields. There are no compiler warnings (using Java 8).

However, all these tests will fail in the second line:

@Test
public void testRedAndCircle() {
    assertThat(Color.RED.getShape()).isNotNull();
    assertThat(Shape.CIRCLE.getColor()).isNotNull(); // fails
}

@Test
public void testCircleAndRed() {
    assertThat(Shape.CIRCLE.getColor()).isNotNull();
    assertThat(Color.RED.getShape()).isNotNull(); // fails
}

@Test
public void testGreenAndTriangle() {
    assertThat(Color.GREEN.getShape()).isNotNull();
    assertThat(Shape.TRIANGLE.getColor()).isNotNull(); // fails
}

@Test
public void testBlueAndSquare() {
    assertThat(Color.BLUE.getShape()).isNotNull();
    assertThat(Shape.SQUARE.getColor()).isNotNull(); // fails
}

How can the null value in the enum field be explained?

It seems that the enum object in the private final fields is not yet completely instantiated.


Solution

  • One way to break the cycle is to make the evaluation lazy in at least one of the enums, thus allowing the other to get initialised before being used. E.g.:

    enum Color {
        RED(() -> Shape.CIRCLE),
        GREEN(() -> Shape.TRIANGLE),
        BLUE(() -> Shape.SQUARE);
    
        private final Supplier<Shape> shape;
    
        Color(Supplier<Shape> shape) {
            this.shape = shape;
        }
    
        Shape getShape() {
            return shape.get();
        }
    }
    
    enum Shape {
        CIRCLE(Color.RED),
        TRIANGLE(Color.GREEN),
        SQUARE(Color.BLUE);
    
        private final Color color;
    
        Shape(Color color) {
            this.color = color;
        }
    
        Color getColor() {
            return color;
        }
    }
    

    But I would strongly advice that you revisit your design, as cyclic dependencies are usually a code smell. If shapes are not of a certain color by their nature and the other way around, you probably need a separate abstraction that defines "red circles" and "blue squares", which has nothing to do with merely being a "circle/square" or being "red/blue".

    enum Color { RED, GREEN, BLUE }
    enum Shape { CIRCLE, TRIANGLE, SQUARE }
    
    enum ColoredShape {
    
        RED_CIRCLE(Shape.CIRCLE, Color.RED),
        GREEN_TRIANGLE(Shape.TRIANGLE, Color.GREEN),
        MAGENTA_TRIANGLE(...) // because why not? :)
        // ......
    
        private final Shape shape;
        private final Color color;
    
    
        ColoredShape(Shape shape, Color color) {
            this.shape = shape;
            this.color = color;
        }
    
        Color color() { return color; }
        Shape shape() { return shape; }
    
    }
    

    This makes it all explicit and renders colors and shapes reusable in contexts other than this very specific one.


    Another major defect of your aproach is that it does not enforce that the pair choices match both ways. I.e. nothing stops me from defining e.g. Color.RED(Shape.CIRCLE) and Shape.CIRCLE(Color.BLUE). So if you want this mapping to be consistent, you should define it in a single place and infer/reuse it the second time (or add some validation logic to ensure consistency).