Java Anti-Patterns – Is Enum Order Sensitivity an Antipattern

anti-patternsenumjava

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?

Best Answer

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#.

Related Solutions

C# – Enum with FlagsAttribute or IEnumerable/ISet

You can create a method that encapsulates enumerating the flags. This way, using flags is almost as easy as an IEnumerable<T>.

One reason why I would prefer flags is what those two approaches mean. With flags, you have a set. With IEnumerable<T>, you have a sequence. That means you have another edge case you have to consider: What if the sequence contains some item more than once?

Java – Several enum classes that declare constants with the same names

This can get confusing... and Magic The Gathering is one that loves to do exceptions to rules. Trying to codify them can often be a challenge.

On the "MonoColored Hybrid" read this and this. And then this for Phyrexian mana.

However, this can boil down to something that is rather sensible.. and doesn't stick too much in a single enum or confuse the next person with an inheritance tree of enums. Enums really 'want' to be simple things, so let them be so. The simpler they are, the easier they are to use, and its a good thing to use enums.

A card has some cost. The good old fashioned lighting bolt is just "R". This is a List of some sort. It could even be empty as in the case of 0 cost artifacts (unless you want to do null object). But its a List.

Each element of the list is a SpellCost object.

A SpellCost would then be composed of:

An EnumMap of costs. The Enum that backs the map would be:

enum Mana {
  WHITE,
  BLUE,
  BLACK,
  RED,
  GREEN,
  COLORLESS
}

And then you would have the EnumMap be as a EnumMap<Mana, Integer>. Something that was a "R/2" for "Red or two colorless" could then be specified in the EnumMap as: mana.put(Mana.RED,1); mana.put(Mana.COLORLESS,2); This also handles cards that are described as "R/W G" (such as Naya Hushblade - because not all cards are monocolored hybrids).

^{_{The EnumMap and EnumSet are two of forgotten collections that are really nice once you realize what they are and do. They give you type safe entries in a map and are backed by either a simple array or a bitfield. The type safety on it can prevent a number of bugs associated with bitfields. And the type safety on the map is something to think about whoever you've got a Map<String, Object> and wonder what happens when there's a typo in the String.}}

Phyrexian mana is then an attribute of the SpellCost. Its a int (though you could do it as a boolean later and yell YAGNI in my general direction). Currently the definition of the cost is "mana or two life" but I wouldn't count on that being the case forever and locking yourself into a boolean now... well.

So, you've got:

public class SpellCost {
    EnumMap<Mana, Integer> mana;
    int lifeCost;

    ...
}

And then write the associated methods for it. You might want to write a container for the cost itself rather than leaving it as a bare List so that you can have appropriate methods against it.

The key design points for this is that it keeps the enum simple as enums aught to be. Any of the complexity of the business logic is kept within the SpellCost class (and possibly the container class too).

Best Answer

Related Solutions

C# – Enum with FlagsAttribute or IEnumerable/ISet

Java – Several enum classes that declare constants with the same names

Related Topic