CODEWITHSUNDEEP
javaenums
Scorcher84
Scorcher84

Reputation: 423

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

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?

Upvotes: 20

Views: 25108

Answers (7)

pvgoddijn
pvgoddijn

Reputation: 12908

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)

Upvotes: 1

Michael Myers
Michael Myers

Reputation: 192015

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.

Upvotes: 14

meriton
meriton

Reputation: 70584

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.

Upvotes: 4

PeterR
PeterR

Reputation: 442

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.

Upvotes: 2

Arne Burmeister
Arne Burmeister

Reputation: 20614

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;
  }

}

Upvotes: 13

toolkit
toolkit

Reputation: 50287

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
}

Upvotes: 17

rsp
rsp

Reputation: 23373

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.

Upvotes: 1

Related Questions