Overriding abstract method or using one single method in enums?

Question

Consider the below enums, which is better? Both of them can be used exactly the same way, but what are their advantages over each other?

1. Overriding abstract method:

public enum Direction {
    UP {
        @Override
        public Direction getOppposite() {
            return DOWN;
        }
        @Override
        public Direction getRotateClockwise() {
            return RIGHT;
        }
        @Override
        public Direction getRotateAnticlockwise() {
            return LEFT;
        }
    },
    /* DOWN, LEFT and RIGHT skipped */
    ;
    public abstract Direction getOppposite();
    public abstract Direction getRotateClockwise();
    public abstract Direction getRotateAnticlockwise();
}

2. Using a single method:

public enum Orientation {
    UP, DOWN, LEFT, RIGHT;
    public Orientation getOppposite() {
        switch (this) {
        case UP:
            return DOWN;
        case DOWN:
            return UP;
        case LEFT:
            return RIGHT;
        case RIGHT:
            return LEFT;
        default:
            return null;
        }
    }
    /* getRotateClockwise and getRotateAnticlockwise skipped */
}

---

Edit: I really hope to see some well reasoned/elaborated answers, with evidences/sources to particular claims. Most existing answers regarding performance isn't really convincing due to the lack of proves.

You can suggest alternatives, but it have to be clear how it's better than the ones stated and/or how the stated ones is worse, and provide evidences when needed.

Answer 1

Forget about performance in this comparison; it would take a truly massive enum for there to be a meaningful performance difference between the two methodologies.

Let's focus instead on maintainability. Suppose you finish coding your Direction enum and eventually move on to a more prestigious project. Meanwhile, another developer is given ownership of your old code including Direction - let's call him Jimmy.

At some point, requirements dictate that Jimmy add two new directions: FORWARD and BACKWARD. Jimmy is tired and overworked and does not bother to fully research how this would affect existing functionality - he just does it. Let's see what happens now:

1. Overriding abstract method:

Jimmy immediately gets a compiler error (actually he probably would've spotted the method overrides right below the enum constant declarations). In any case, the problem is spotted and fixed at compile time.

2. Using a single method:

Jimmy doesn't get a compiler error, or even an incomplete switch warning from his IDE, since your switch already has a default case. Later, at runtime, a certain piece of code calls FORWARD.getOpposite(), which returns null. This causes unexpected behavior and at best quickly causes a NullPointerException to be thrown.

Let's back up and pretend you added some future-proofing instead:

default:
    throw new UnsupportedOperationException("Unexpected Direction!");

Even then the problem wouldn't be discovered until runtime. Hopefully the project is properly tested!

Now, your Direction example is pretty simple so this scenario might seem exaggerated. In practice though, enums can grow into a maintenance problem as easily as other classes. In a larger, older code base with multiple developers resilience to refactoring is a legitimate concern. Many people talk about optimizing code but they can forget that dev time needs to be optimized too - and that includes coding to prevent mistakes.

Edit: A note under JLS Example §8.9.2-4 seems to agree:

Constant-specific class bodies attach behaviors to the constants. [This] pattern is much safer than using a switch statement in the base type... as the pattern precludes the possibility of forgetting to add a behavior for a new constant (since the enum declaration would cause a compile-time error).

Answer 2

I actually do something different. Your solutions have setbacks: abstract overridden methods introduce quite a lot of overhead, and switch statements are pretty hard to maintain.

I suggest the following pattern (applied to your problem):

public enum Direction {
    UP, RIGHT, DOWN, LEFT;

    static {
      Direction.UP.setValues(DOWN, RIGHT, LEFT);
      Direction.RIGHT.setValues(LEFT, DOWN, UP);
      Direction.DOWN.setValues(UP, LEFT, RIGHT);
      Direction.LEFT.setValues(RIGHT, UP, DOWN);
    }

    private void setValues(Direction opposite, Direction clockwise, Direction anticlockwise){
        this.opposite = opposite;
        this. clockwise= clockwise;
        this. anticlockwise= anticlockwise;
    }

    Direction opposite;
    Direction clockwise;
    Direction anticlockwise;

    public final Direction getOppposite() { return opposite; }
    public final Direction getRotateClockwise() { return clockwise; }
    public final Direction getRotateAnticlockwise() { return anticlockwise; }
}

With such design you:

• never forget to set a direction, because it is enforced by the constructor (in case case you could)

• have little method call overhead, because the method is final, not virtual

• clean and short code

• you can however forget to set one direction's values

Answer 3

First variant is faster and is probably more maintainable, because all properties of the direction are described where the direction itself is defined. Nevertheless, putting non-trivial logic into enums looks odd for me.

Answer 4

The second variant will probably be a little bit faster as the >2-ary polymorphism will force a full virtual function call on the interface, vs a direct call and index for the latter.

The first form is the object-oriented approach.

The second form is a pattern-matching approach.

As such the first form, being object-oriented, makes it easy to add new enums, but hard to add new operations. The second form does the opposite

Most experienced programmers I know would recommend using pattern-matching over object-orientation. As enums are closed, adding new enums is not an option; therefore, I would definitely go with the latter approach myself.

Answer 5

The enum values can be considered as independant classes. So considering the Object Oriented Concepts each enum should define its own behaviour. So i would reccommend first approach.

Answer 6

The first version is probably much faster. The Java JIT compiler can apply aggressive optimizations to it because enums are final (so all methods in them are final, too). The code:

Orientation o = Orientation.UP.getOppposite();

should actually become (at runtime):

Orientation o = Orientation.DOWN;

i.e. the compiler can remove the overhead for the method call.

From a design perspective, it's the proper way to do these things with OO: Move knowledge close to the object that needs it. So UP should know about it's opposite, not some code elsewhere.

The advantage of the second method is that it's more readable since all related things are grouped better (i.e. all the code related to "opposite" is in one place instead of a bit here and a bit there).

EDIT My first argument depends on how smart the JIT compiler is. My solution for the problem would look like this:

public enum Orientation {
    UP, DOWN, LEFT, RIGHT;

    private static Orientation[] opposites = {
        DOWN, UP, RIGHT, LEFT
    };

    public Orientation getOpposite() {
        return opposites[ ordinal() ];
    }
}

This code is compact and fast, no matter what the JIT can or could do. It clearly communicates intent and, given the rules of ordinals, it will always work.

I would also suggest to add a test which makes sure that when calling getOpposite() for each value of the enum, you always get a different result and none of the results is null. That way, you can be sure that you got every case.

The only problem left is when you change the order of values. To prevent problems in this case, assign each value an index and use that to look up values in an array or even in Orientation.values().

here is another way to do it:

public enum Orientation {
    UP(1), DOWN(0), LEFT(3), RIGHT(2);

    private int opposite;

    private Orientation( int opposite ) {
        this.opposite = opposite;
    }

    public Orientation getOpposite() {
        return values()[ opposite ];
    }
}

I don't like this approach, though. It's too hard to read (you have to count the index of each value in your head) and too easy to get wrong. It would need a unit test per value in the enum and per method that you can call (so 4*3 = 12 in your case).

Answer 7

You could also simpy implement it once like this (you need to keep the enum constants in the appropriate order):

public enum Orientation {

    UP, RIGHT, DOWN, LEFT; //Order is important: must be clock-wise

    public Orientation getOppposite() {
        int position = ordinal() + 2;
        return values()[position % 4];
    }
    public Orientation getRotateClockwise() {
        int position = ordinal() + 1;
        return values()[position % 4];
    }
    public Orientation getRotateAnticlockwise() {
        int position = ordinal() + 3; //Not -1 to avoid negative position
        return values()[position % 4];
    }
}

Answer 8

Answer: It Depends

1. IF your method definitions are simple

   This is the case with your very simple example methods, which just hard-code an enum output for each enum input

   • implement definitions specific to an enumeration value right next to that enumeration value
   • implement definitions common to all enumeration values at the bottom of the class in the "common area"; if the same method signature is to be available for all enum values but none/part of logic is common, use abstract method definitions in the common area

   i.e. Option 1

   Why?

   • readability, consistency, maintainability: the code directly related to a definition is right next to the definition
   • compile-time checking if abstract methods declared in common area, but not specified in enum value area

   Note that the North/South/East/West example could be considered to represent a very simple state (of current direction) and the methods opposite/rotateClockwise/rotateAnticlockwise could be considered to represent user commands to change state. Which raises the question, what do you do for a real-life, typically complex state machine??

2. IF your method definitions are complex:

   State-Machines are often complex, relying on current (enumerated value) state, command input, timers, and a fairly large number rules and business exceptions to determine the new (enumerated value) state. Other rare times, methods may even determine enumerated value output via calculations (e.g. scientific/engineering/insurance rating categorisation). Or it could use data-structures such as a map, or a complex data structure suited to an algorithm. When the logic is complex then extra care is required and the balance between "common" logic and "enum value-specific" logic changes.

   • avoid putting excessive code volume, complexity, and repeated 'cut & paste' sections right next to the enum value
   • try to refactor as much logic as possible into the common area - possibly putting 100% of logic here, but if not possible, employing the Gang Of Four "Template Method" pattern to maximise the amount of common logic, but flexibly allow a small amount of specific logic against each enum value. i.e. As much as possible of Option 1, with a little of Option 2 allowed

   Why?

   • readability, consistency, maintainability: avoids code bloat, duplication, poor textual formatting with masses of code interspersed amongst enum values, allows the full set of enum values to be quickly seen and understood

   • compile-time checking if using Template Method pattern and abstract methods declared in common area, but not specified in enum value area

   • Note: you could put ALL logic into a separate helper class, but I personally don't see any advantages to this (not performance/maintainability/readability). It breaks encapsulation a little and once you have all the logic in one place, what difference does it make to add a simple enum definition back to the top of the class? Splitting code across multiple classes is a different matter and is to be encouraged where appropriate.