Direction enum class

Question

As request of Simon's comment:

Direction.java

package net.woopa.dungeon.datatypes;

import java.util.Random;

public enum Direction {
    NORTH, EAST, SOUTH, WEST;
    private static Random rnd = new Random();

    static public Direction randomDirection() {
        return Direction.values()[rnd.nextInt(4)];
    }

    // Rotate 90 degrees clockwise
    public Direction rotate90() {
        return values()[(ordinal() + 1) % 4];
    }

    // Rotate 180 degrees
    public Direction rotate180() {
        return values()[(ordinal() + 2) % 4];
    }

    // Rotate 270 degrees clockwise (90 counterclockwise)
    public Direction rotate270() {
        return values()[(ordinal() + 3) % 4];
    }

    public Boolean isHorizontal() {
        return this == EAST || this == WEST;
    }

    public Boolean isVertical() {
        return this == NORTH || this == SOUTH;
    }

    public int dx(int steps) {
        if (this == EAST) {
            return steps;
        }
        if (this == WEST) {
            return -steps;
        }
        return 0;
    }

    public int dy(int steps) {
        if (this == NORTH) {
            return steps;
        }
        if (this == SOUTH) {
            return -steps;
        }
        return 0;
    }

    public int forwards_x(int n) {
        if (this == EAST) {
            return n + 1;
        }
        if (this == WEST) {
            return n - 1;
        }
        return n;
    }

    public int forwards_y(int n) {
        if (this == NORTH) {
            return n + 1;
        }
        if (this == SOUTH) {
            return n - 1;
        }
        return n;
    }

    public int backwards_x(int n) {
        if (this == EAST) {
            return n - 1;
        }
        if (this == WEST) {
            return n + 1;
        }
        return n;
    }

    public int backwards_y(int n) {
        if (this == NORTH) {
            return n - 1;
        }
        if (this == SOUTH) {
            return n + 1;
        }
        return n;
    }

    public int left_x(int n) {
        if (this == NORTH) {
            return n - 1;
        }
        if (this == SOUTH) {
            return n + 1;
        }
        return n;
    }

    public int left_y(int n) {
        if (this == EAST) {
            return n + 1;
        }
        if (this == WEST) {
            return n - 1;
        }
        return n;
    }

    public int right_x(int n) {
        if (this == NORTH) {
            return n + 1;
        }
        if (this == SOUTH) {
            return n - 1;
        }
        return n;
    }

    public int right_y(int n) {
        if (this == EAST) {
            return n - 1;
        }
        if (this == WEST) {
            return n + 1;
        }
        return n;
    }
}

What are your thoughts about this class? The main grip I have personally is the left_y, right_x, etc methods. The code works, and here is some example of its usage below. But I feel there may be a better way?

Example usage:

Direction dir = Direction.randomDirection();
if (grid.get(dir.left_x(x), dir.left_y(y)).equals(CoreMaterial.WALL) && grid.get(dir.right_x(x), dir.right_y(y)).equals(CoreMaterial.WALL) && grid.get(dir.backwards_x(x), dir.backwards_y(y)).equals(CoreMaterial.FLOOR) && grid.get(dir.forwards_x(x), dir.forwards_y(y)).equals(CoreMaterial.FLOOR)) {
    StandardMethods.build_door(dir, new Vector2D(x, y), CoreMaterial.CAKE, grid);
}

The above example has a location (x,y) for a door on a grid and then checks around it to ensure that there are doors either side and floors the other two either sides?

---

Symbols:

. = floor

# = wall

+ = door

Example:

.#. 
.+.
.#.

This would be okay for the placing of the door.

Answer 1

There are two things I think worth mentioning here:

• use of random
• magic numbers in enums

Random

Even though the java.util.Random class is thread-safe, it is not a great idea to use them in a multi-threaded context. As your class is an enum, it is likely that at some point it may be used in a threaded context (if it is not already). You should consider changing your code:

private static Random rnd = new Random();

to instead be (note also that this is 'final', and this is a java.util.concurrent.ThreadLocalRandom):

private static final ThreadLocalRandom rnd = ThreadLocalRandom.current();

This will give you better scalability in the future (or the present if you are already multi-threaded.

Enum Magic Numbers

Enums are a great concept in Java, and, it is one of those places where I believe magic numbers (special values with little apparent context or meaning) are really useful, and are OK.

You can embed 'magic' in with the enums and not worry too much about the readability... enums are supposed to be magic things.... So, I would consider code like the following:

import java.util.concurrent.ThreadLocalRandom;

public enum Direction {

    // use magic numbers to set the ordinal (used for rotation),
    // and the dx and dy of each direction.
    NORTH(0, 0, 1),
    EAST(1, 1, 0),
    SOUTH(2, 0, -1),
    WEST(3, -1, 0);

    private static final ThreadLocalRandom rnd = ThreadLocalRandom.current();

    static public Direction randomDirection() {
        return Direction.values()[rnd.nextInt(4)];
    }

    private final int r90index, r180index, r270index;
    private final boolean horizontal, vertical;
    private final int dx, dy;

    private Direction(int ordinal, int dx, int dy) {
        // from the ordinal, dx, and dy, we can calculate all the other constants.
        this.dx = dx;
        this.dy = dy;
        this.horizontal = dx != 0;
        this.vertical = !horizontal;
        this.r90index  = (ordinal + 1) % 4; 
        this.r180index = (ordinal + 2) % 4; 
        this.r270index = (ordinal + 3) % 4; 
    }


    // Rotate 90 degrees clockwise
    public Direction rotate90() {
        return values()[r90index];
    }

    // Rotate 180 degrees
    public Direction rotate180() {
        return values()[r180index];
    }

    // Rotate 270 degrees clockwise (90 counterclockwise)
    public Direction rotate270() {
        return values()[r270index];
    }

    public Boolean isHorizontal() {
        return horizontal;
    }

    public Boolean isVertical() {
        return vertical;
    }

    public int dx(int steps) {
        return dx * steps;
    }

    public int dy(int steps) {
        return dy * steps;
    }

    public int forwards_x(int n) {
        return n + dx;
    }

    public int forwards_y(int n) {
        return n + dy;
    }

    public int backwards_x(int n) {
        return n - dx;
    }

    public int backwards_y(int n) {
        return n - dy;
    }

    public int left_x(int n) {
        // if we are E/W facing, our left/right 'x' co-ords are still 'x'
        // if we are N/S facing, our left/right 'x' is `-dy`
        return n - dy;
    }

    public int left_y(int n) {
        // see left_x comments for the idea
        return n + dx;
    }

    public int right_x(int n) {
        // see left_x comments for the idea
        return n + dy;
    }

    public int right_y(int n) {
        // see left_x comments for the idea
        return n - dx;
    }
}

The code above is very efficient. There are no conditionals in the methods, they will likely all be inlined in to the calling code. The 'smarts' are all contained within the enum, it 'just works'.

Answer 2

It is better to replace

return Direction.values()[rnd.nextInt(4)];

with something like

return Direction.values()[rnd.nextInt(Direction.values().length)];

for it to not break if you decide to add something like NORTH_EAST.

I don't understand what functions like dx(int steps) are supposed to do, but it could be much easier to represent a direction as a vector. For example, rotation becomes simple multiplication, your methods are much more readable and robust. Also I don't understand why do you not use a Point-like class.

Also it is not logical for a direction to house its very own object of Random class.

Answer 3

An enum is a nice simple way to represent cardinal directions. No problem there. All the functions you've provided are valuable to consumers. No problem there.

But within the class, when you implement all those functions, the enum is getting in the way and making things complicated. The enum is there for your users, not for you. I would suggest you internally use a representation that makes more sense, like 2D coordinates. You have a lot of math that will make more sense with 2D coordinates.

A simple way to do that is to translate enums to coordinates for internal math, and back to enums for results.

private int[][] coordinates = new int[][] {
  new int[] {0,1,  0},  // NORTH (the third element maps back to the enum)
  new int[] {1,0,  1},  // EAST
  new int[] {0,-1, 2},  // SOUTH
  new int[] {-1,0, 3}   // WEST
};
public int[] toCoordinates(Direction d) {
  return coordinates[d.ordinal()];
}
public static Direction coordinatesToEnum(int[] c) {
  return Direction.values(c[2]);
}

public Boolean isVertical() {
  return this.toCoordinates()[1] == 0;
}
public Direction rotate180() {
  int[] c = this.toCoordinates();
  int result = new int[] { c[0]*-1, c[1]*-1 };
  return coordinatesToEnum(result);
}
// and so forth.

(This also allows your users to convert easily to coordinate system if that's better for them. Up to you whether you document that.)

Answer 4

I don't think that a Direction enum is sufficient to model your problem. Direction should just be a list of four cardinal directions. What you need is a Cursor or Character object that represents a position and heading on a grid. (Unfortunately, Character clashes with java.lang.Character, so you'll need to pick a different name.)

Your code should look more or less like this. Design your Cursor class accordingly to make it work.

Cursor cur = new Cursor(grid, x, y, Direction.random());
if ( CoreMaterial.WALL.equals(cur.getLeft()) &&
     CoreMaterial.WALL.equals(cur.getRight()) &&
     CoreMaterial.FLOOR.equals(cur.getBack()) &&
     CoreMaterial.FLOOR.equals(cur.getFront()) ) {
    StandardMethods.buildDoor(cur, CoreMaterial.CAKE);
}

When you're done, the Direction enum should be pretty minimal — just a list of four cardinal directions and a .random() static method.

Answer 5

Rename rotate90, rotate180, and rotate270 as right, reverse, and left, respectively. Don't use degrees unless you have to, and in this case they have nothing to do with this geometry system.