Skip to main content
added 3 characters in body
Source Link
rolfl
  • 98.2k
  • 17
  • 220
  • 419
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;
    }
}
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;
    }
}
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;
    }
}
not -> note, fixed numbers
Source Link
rolfl
  • 98.2k
  • 17
  • 220
  • 419

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 (even ifif it is not already) be used in a threaded context. You should consider changing your code:

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

import java.util.concurrent.ThreadLocalRandom;

@SuppressWarnings("javadoc")
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, -10, 0-1),
    WEST(3, 0, -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;
    }
}

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 (even if it is not already) be used in a threaded context. You should consider changing your code:

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

import java.util.concurrent.ThreadLocalRandom;

@SuppressWarnings("javadoc")
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, -1, 0),
    WEST(3, 0, -1);
    
    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;
    }
}

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:

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

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;
    }
}
Source Link
rolfl
  • 98.2k
  • 17
  • 220
  • 419

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 (even if it is not already) be used in a threaded context. You should consider changing your code:

private static Random rnd = new Random();

to instead be (not 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;

@SuppressWarnings("javadoc")
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, -1, 0),
    WEST(3, 0, -1);
    
    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'.