9

Is it an anti-pattern to depend on a particular order of an enum's instance declarations? For example, consider:

public enum CompassPoint {
    North,
    East,
    South,
    West;
}

These points are in clockwise order. Is there a good reason to not have code rely on this fact? More specifically, is it "OK" to rely on this code:

 CompassPoint from;
 int toRightIndex = (from.ordinal() + 1) % CompassPoint.values();
 CompassPoint toRight = CompassPoint.values()[toRightIndex];

"OK" here is "best practice", including avoiding a time bomb if someone innocently added instances for the quarter points, which may (or may not) break the intention of the meaning of "to right".

If "not OK", is it less "not OK" if such code is restricted to being internal to the enum class?

3
  • Is it OK to depend on the order of an array? It has to be in order somewhere. – paparazzo Jul 26 '15 at 6:18
  • 6
    In that case, where is the comment in your enum that tells other developers not to change the order? Adding such a trap without any warning to your fellow developers is a huge anti pattern. – gnasher729 Jul 26 '15 at 8:29
  • @gnasher729 Shouldn't the tests cover that? – John MacIntyre Aug 10 '15 at 10:06
17

I'd also say, you mostly answered the question by your own. I wouldn't implement it that way because it is brittle.

When having that code as part of the enum class itself, it is certainly better because the problem is isolated to the implementation detail of the enum. For the problem at hand I think that solution would be 'good enough' and therefore acceptable.

When assuming that the enum will be used frequently and is likely to be extended in the future, I'd make use of the advanced Java enum features like fields and methods:

    public enum CompassDirection {
    NORTH(0),
    EAST(90),
    SOUTH(180),
    WEST(270);

    private final double degreesFromNorth;

    CompassDirection(double degreesFromNorth) {
        this.degreesFromNorth = degreesFromNorth;
    }

    public CompassDirection directionToRight(){
        directionForDegreesFromNorth(degreesFromNorth + 90);
    }

    public CompassDirection directionToLeft(){
        directionForDegreesFromNorth(degreesFromNorth - 90);
    }

    public CompassDirection directionForDegreesFromNorth(double degreesFromNorth){
        double normalizedDegrees = Math.floorMod(degreesFromNorth, 360);
        for(CompassDirection direction : CompassDirection.values()){
            if(direction.degreesFromNorth == normalizedDegrees){
                return direction;
            }
        }

        throw new Exception("No matching direction could be found");
    }
}

Java enums are really powerful and I must say I miss them badly when programming C#.

5
  • 3
    Math.abs(degreesFromNorth % 360) is incorrect since it returns 90 when going to left from north (that is -90). Correct code is (degreesFromNorth+360)%360 – aragaer Jul 26 '15 at 11:38
  • 2
    @aragaer: Or better yet, Math.floorMod(degreesFromNorth) – Eric Jul 26 '15 at 18:07
  • 1
    Good Point @aragaer & Eric. I adjusted the code accordingly. Didn't know floorMod, but its a perfect fit. – Stefan Sieber Jul 26 '15 at 18:16
  • This feels right - that is, if there's an attribute about the instance use that in preference to ordinal(). Also, there's a way to implement directionForDegreesFromNorth() without looping through the values by using a Map that is populated from the constructor. Before you say "but static initialization order prevents this", there is a way (have a try at it). Also, instead of Math.floorMod() you can just (degreesFromNorth + 360) % 360 – Bohemian Jul 26 '15 at 23:52
  • When keeping directionForDegreesFromNorth public I'd rather use Math.floorMod because it will work beyond -360 degrees. For such small numbers of entries looping should not have a noticeable performance impact. – Stefan Sieber Jul 27 '15 at 10:13
4

I think you've pretty much answered your own question :)

The major reason not to rely on the ordinal in code external to the enum, is dealing with change. Like you've pointed out, adding a NorthEast compass point might alter the meaning of your 'toRightIndex' calculation. If you've written code that relies on the ordinal, that will probably need to change. So it makes sense not to rely on the ordinal outside the enum.

That said, I can't see any practical drawback in using the ordinal in a method internal to the enum - provided that to the people maintaining and using the code, the ordinal represents an easily-understood order that makes sense in the real world.

A bit of advice from the javadocs for Enum:

public final int ordinal()
Returns the ordinal of this enumeration constant (its position in its enum declaration, where the initial constant is assigned an ordinal of zero). Most programmers will have no use for this method. It is designed for use by sophisticated enum-based data structures, such as EnumSet and EnumMap.

2

It makes sense to place enum into some logical order if for no other reason than for the sake of making sense of them. For example, would you have not put an enum of months with February after October? (rhetorical question).

In several langauges enums can be treated as having an order, much like the example you provide. For that reason it makes great sense to put them in logical or perhaps more correctly 'real world order'.

From the example you provide, not all cultures necessary take things in 'clockwise' order, in fact some cultures inherently use 'counter-clockwise' ordering. In these cultures you will find enums which represent things compass direction to be in 'counter-clockwise' order. Something such as months which do not have a clockwise/counterclockwise idea are still found in ordinal order.

Another consideration about order comes when you consider sorting. If you would sort North before East then that should be the order used to fall inline with default sorting algorithms that might sort things using the enum as the key.

When you write code you should take into consideration your own expectations or those who might use your code. If you know and expect your enums to follow clockwise order, then you should use that as best practice. If you are writing code to be used by people who typically use counterclockwise order then you may want to order them that way instead. It might also be argued that since most cultures use clockwise rotation then that is what you should use and rely on.

No matter what you do, some sense of order should be maintained and should be kept consistent across all your code, if you use clockwise, then use it everywhere. I wouldn't, however, do something like enum { North, South, East, West } as that has no typical ordering.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.