Random walk on a 2D grid

The program assignment:


A drunkard in a grid of streets randomly picks one of four directions
and stumbles to the next intersection, then again randomly picks one
of four directions, and so on. You might think that on average the
drunkard doesn't move very far because the choices cancel each other
out, but that is not the case. Represent locations as integer pairs
(x,y). Implement the drunkard's walk over 100 intersections, starting
at (0,0) and print the ending location.

I created this Drunkard class:

import java.util.*;

class Drunkard {
    int x, y; 
    Drunkard(int x, int y) {
    this.x = x; 
    this.y = y;
    } 
    void moveNorth() {
    this.y -= 1; 
    }
    void moveSouth() {
    this.y += 1; 
    }
    void moveEast() {
    this.x += 1; 
    }
    void moveWest() {
    this.x -= 1; 
    }
    void report() {
    System.out.println("Location: " + x + ", " + y); 
    } 
}

And this DrunkardTester class to test the above class

import java.util.Random;

public class DrunkardTester {
    public static void main(String[] args) {
    Random generator = new Random(); 
    Drunkard drunkard = new Drunkard(0, 0); 
    int direction; 
    for (int i = 0; i < 100; i++) {
        direction = Math.abs(generator.nextInt()) % 4; 
        if        (direction == 0) { // N
        drunkard.moveNorth();
        } else if (direction == 1) { // E
        drunkard.moveEast(); 
        } else if (direction == 2) { // S
        drunkard.moveSouth();
        } else if (direction == 3) { // W
        drunkard.moveWest();
        } else {
        System.out.println("Impossible!"); 
        }      
    } 
    drunkard.report();
    }

}

It complies and runs fine, and it prints a different coordinate location each time I run it, but I just wanted to get a few other pairs of eyes on it to see if there are any glaring errors.

Also, on my DrunkardTester class is there anyway to eliminate the "Impossible!" else statement? I wasn't really

There is absolutely no need to call Math.abs for generator.nextInt() since nextInt will never produce a negative number. Also, your last else-statement is impossible to happen so you can erase that one.

There is also no need to use the modulo (%) operator to fix your direction variable to the correct range when you instead can supply an integer value to the nextInt method. generator.nextInt(4); will give you a value from 0 to 3 (inclusive).

It is a Java coding convention to indent your code properly, after each { the next line should have one more indentation step, like this:

void moveSouth() {
    this.y += 1; 
}

The below might be a little bit over your level, but it is a really nice solution and is very useful to learn. When you are dealing with four directions, I would create an enum to help with the different direction possibilities.

public enum Direction4 {
    NORTH(0, -1), EAST(1, 0), SOUTH(0, 1), WEST(-1, 0);

    private final int dx;
    private final int dy;

    private Direction4(int dx, int dy) {
        this.dx = dx;
        this.dy = dy;
    }
    public int getX() { return dx; }
    public int getY() { return dy; }
}

Using this enum, you can simplify several things of your other code. For starters, you can replace your move-methods with one single move method:

void move(Direction4 direction) {
    this.x += direction.getX();
    this.y += direction.getY();
}

And when generating the direction to move at, you can use the Direction4 enum:

// Get an index number and input the "size" of the enum to `nextInt`
int directionIndex = generator.nextInt(Direction4.values().length);

// Get the direction from the index generated above
Direction4 dir = Direction4.values()[directionIndex];

// Make the move:
drunkard.move(dir);

It might be subjective whether four different moveNorth, moveSouth etc. methods are preferable instead of using a single move-method that takes the direction as an argument. I myself find that using an enum redures the amount of code and it makes the code flexible and scalable (if you want to add or modify existing directions you can do so, for example adding MEGA_NORTH that moves 10 steps north just for fun).

If you don't perform the enum approach, then at least use a switch

direction = generator.nextInt(4);
switch (direction) {
    case 0:
        drunkard.moveNorth();
        break;
    case 1:
        drunkard.moveEast();
        break;
    case 2:
        drunkard.moveSouth();
        break;
    case 3:
        drunkard.moveWest();
        break;
}

Although I must say: There's not much that beat a nice little enum :)

StackExchange Code Review Q#34054, answer score: 9