Is it bad practice to use an Enum's ordinal value to index an array in Java?
Asked Answered
L

7

20

I have two arrays: Walls and Neighbors.

public boolean[] walls = new boolean[4];
public Cell[] neighbors = new Cell[4];

and I have an Enum:

enum Dir
{
    North,
    South,
    East,
    West
}

Now, I would like to be able to access walls or neighbors by their direction, so I don't have to pass around a bunch of magic indexes.

However, when I was reading the documentation for Enum.ordinal() it said that programmers will have almost no use for this method which made me think it shouldn't be used in this way.

I was thinking of doing something like:

    List<Dir> availableDirections = new ArrayList<Dir>();
    for(Dir direction : Dir.values())
        if (!Neighbors[direction.ordinal()].Visited)
            availableDirections.add(direction);

or even:

return Neighbors[Dir.North.ordinal()];

Should I revert to using static constants for NORTH, SOUTH, EAST, WEST with the index value set to them or use an Enum's ordinal method?

Lobster answered 15/10, 2009 at 19:37 Comment(0)
E
17

On a tangential issue, it might be better to use an EnumMap for your neighbours:

Map<Dir, Cell> neighbours = 
    Collections.synchronizedMap(new EnumMap<Dir, Cell>(Dir.class));

neighbours.put(Dir.North, new Cell());

for (Map.Entry<Dir, Cell> neighbour : neighbours.entrySet()) {
    if (neighbour.isVisited()) { ... }
}

etc..

BTW: Enum instances should by convention be all caps,

enum Dir {
    NORTH,
    EAST, 
    SOUTH,
    WEST
}
Entopic answered 15/10, 2009 at 19:42 Comment(1)
And EnumSet for walls. Probably don't need it to be synchronised.Keelin
N
14

The documentation only says that most programmers will have no use for the method. This is one case of legitimate use. Assuming your class controls both the enum and the array, there is no reason to fear the ordinal() method for indexing the array (since you can always keep them in sync).

However, if your usage gets any more complicated, you will likely want to use an EnumMap instead, as has been suggested.

Napery answered 15/10, 2009 at 19:41 Comment(1)
though this is a valid case with the code shown, most code gets changed sometime which can easily make it a invalid situation for it.Skive
V
13

You can also enhance an enum (index clockwise):

enum Dir
{
  NORTH(0),
  SOUTH(2),
  EAST(1),
  WEST(3);

  private final int index;

  private Dir(int index) {
    this.index = index;
  }

  public int getIndex() {
    return index;
  }

}
Vaish answered 15/10, 2009 at 19:50 Comment(0)
H
4

The JavaDoc says

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.

I think they want to say that most programmers will prefer using an EnumMap or EnumSet over manually indexing into an array. They certainly don't mean that you should substitute a couple of integer variables for your enum, as you would lose type safety in doing so.

If you need the flexibility to be able to reorder the enum constants without affecting the order in the array you can isolate the index into another field of the enum as has been described by Arne.

Hemicycle answered 15/10, 2009 at 20:4 Comment(0)
C
2

If you are not persisting the arrays or in any other way are making yourself dependent on different versions of your enum class, it's safe to use ordinal().

If you want don't want to rely on the implicit ordering of the enum values, you could introduce a private index value:

public enum Direction {
  NORTH(0),
  SOUTH(1),
  EAST(2),
  WEST(3);

  private int _index;

  private Direction (int index_)
  {
    _index = index_;
  }

  public int getIndex()
  {
    return _index;
  }
}

From here it's easy to both allow for easy lookup of index to Direction (By creating a Map in a static block for compact persistence; Do uniqueness check in static block etc.

Consignor answered 15/10, 2009 at 19:56 Comment(0)
M
1

Using an enum's ordinal for your use depends on implicit order. I like to be explicit especially if you use the integer values as index into your array linking value to meaning.

In this case I would therefore opt to use final static int NORTH = 0, etc.

Midshipman answered 15/10, 2009 at 19:47 Comment(0)
S
1

i strongly recommend against it because the Ordinal value is based on the order in your java code.

The use of ordinal() will make you code very hard to maintain, especially if some form of persistence enters the equation.

for example if you decide to add diagonal directions like NORTH_WEST, SOUTH_EAST. if you are using ordinal(), you must add these at the bottom of your list or else what used to be SOUTH might become NORTH.

ie you can change your enyum to without (possibly) changing functionality

N  (is now 0 was 0) 
NE (is now 1) 
E (is now 2 was 1) 
SE (is now 3 ) 
S  (is mow 4 was 2) 
SW (is now 5) 
W  (is now 6 was 3)
Skive answered 15/10, 2009 at 23:1 Comment(0)

© 2022 - 2024 — McMap. All rights reserved.