Langton's Ant cellular automaton in Java

Question

I made a couple of little games and cellular automata in Java. For every new 'game', I used the same code as a base, except for the game logic of course. With every iteration of me customizing my existing code to my new idea, my coding skills have much improved and I added new things that I learned. Which is quite a lot.

So I now arrived at the point where I have to start thinking about efficiency, code length but also start to get used to inheritance, encapsulation and most importantly conventions.

This Langton's ant cellular automata got to the point of working pretty well, so I now wanted to see how I can improve such code. But after looking over the code for a while, I realized that maybe it would be better to just rewrite everything. There are 10 classes, so I put the code on GitHub: https://github.com/plankins/LangtonsAnt

Some information about the code:

The rules for the ant are in the Game class:

  public void step() {
    Point2 p = ant.getPos();
    int val = board.getOnBoard(p);
    
    switch (val) {
        case 0 -> ant.turnRight();
        case 1 -> ant.turnRight();
        case 2 -> ant.turnLeft();
        case 3 -> ant.turnRight();
    }
    
    ant.moveForward();
    stepCnt++;
}

Where val is the tile the ant is currently on. After the ant moves forward, the tile is updated in the Board class:

    public void updateTile(Point2 p) {
    int var = getOnBoard(p);
    int newVar = var+1;
    if(newVar > 3){
        newVar = 0;
    }
    setOnBoard(p, newVar);
}

Controls are handled in the InputHandler class, which calls onAction with the enums in Action.java.

They are:

• Space: Pause/Start simulation
• S: Perform one step
• Right/Left Keys: Increase/Decrease steps per frame
• Up/Down Keys: Increase/Decrease delay between each actual step
• P: Prints whole array to console (just for debugging, on much smaller grids)
• C: Clears board
• R: Resets ant's position
• Mouse Buttons: Draw and erase cells with the type of cell that is selected (makes not much sense in Langton's ant, but kept it anyway)
• 1-5 Numbers: Select the colour of cells to draw

I know I haven't been very consistent with conventions and such, but would love to hear what others think about my code.

• What should I do differently?
• How can I improve efficiency? (with efficiency meaning both computational and code length efficiency)

I am planning on moving to JOGL or LWJGL and change my board class to allow for an infinite (a very large) grid and zooming/moving the grid. I briefly looked at HashMaps and sets and that kind of stuff and I feel that thats what I should use. If not, how could I improve this grid situation?

Thank you to everyone who takes the time to review my code!

EDIT: These are the major classes I have 'concerns' about, sorry for misunderstanding the FAQ.

Game.java

public class Game {

    private Board board;
    private static Game game;
    public GameFrame frame;

    public Loop loop;
    public Ant ant;

    public int SIZE;
    public int WINDOW_SIZE;
    public int GRID_SIZE;
    public int mouseX = 0;
    public int mouseY = 0;
    public int slot = 1;
    public int stepCnt = 0;

    boolean running = false;

    public Game(int windowSize, int gridSize) {
        WINDOW_SIZE = windowSize;
        GRID_SIZE = gridSize;
        SIZE = WINDOW_SIZE / GRID_SIZE;
        game = this;
        board = new Board(this);
        ant = new Ant(gridSize / 2, gridSize / 2, 'u', this);
        loop = new Loop(this);
        frame = new GameFrame(this);

        loop.run();
    }
    
    public void drawOnBoard(Action a, int x, int y) {
        switch (a) {
            case L_MOUSE:
                switch (slot) {
                    //draw on the grid with a total of 6 tile states (including 0), but right now only 4 used.
                    case 1 -> board.setOnBoard(x, y, 1);
                    case 2 -> board.setOnBoard(x, y, 2);
                    case 3 -> board.setOnBoard(x, y, 3);
                    case 4 -> board.setOnBoard(x, y, 4);
                    case 5 -> board.setOnBoard(x, y, 5);
                }
                break;
            case R_MOUSE:
                //"erase", if you will.
                board.setOnBoard(x, y, 0);
                break;
            case MID_MOUSE:
                //set the ant at mouse pos.
                ant.getPos().set(x, y);
        }
    }

    public void step() {
        Point2 p = ant.getPos();
        int val = board.getOnBoard(p);
        switch (val) {
            //by changing this, one can change the pattern the ant produces.
            case 0 -> ant.turnRight();
            case 1 -> ant.turnLeft();
            case 2 -> ant.turnRight();
            case 3 -> ant.turnLeft();
        }
        ant.moveForward();
        stepCnt++;
    }
    
    //every user input is handled by this function, with Action enums.
    public void onAction(Action a, AWTEvent e) {
        System.out.println(a);
        if (e instanceof MouseEvent) {
            mouseX = Math.floorDiv(((MouseEvent) e).getX(), game.SIZE);
            mouseY = Math.floorDiv(((MouseEvent) e).getY(), game.SIZE);
            // idek why i used math.floordiv
            drawOnBoard(a, mouseX, mouseY);
            if (e instanceof MouseWheelEvent) {
                switch (a) {
                    //nothing here yet
                    case WHEEL_DOWN -> {
                    }
                    case WHEEL_UP -> {
                    }
                }
            }
        } else if (e instanceof KeyEvent) {
            double increment = loop.stepDelay / 10;
            switch (a) {
                case SPACE -> game.toggleSimulation();
                case S -> game.step();
                case C -> {
                    board.clearArray();
                    stepCnt = 0;
                }
                case UP -> {
                    if (loop.stepDelay + increment <= 2000) {
                        loop.stepDelay += increment;
                    }
                }
                case DOWN -> {
                    if (loop.stepDelay - increment > 0) {
                        loop.stepDelay -= increment;

                    }
                }
                case RIGHT -> {
                    if (loop.stepsPerLoop <= 400000)
                        loop.stepsPerLoop += 1 + loop.stepsPerLoop / 10;
                }
                case LEFT -> {
                    if (loop.stepsPerLoop >= 0)
                        loop.stepsPerLoop -= 1 + loop.stepsPerLoop / 10;
                }
                case N1 -> this.slot = 1;
                case N2 -> this.slot = 2;
                case N3 -> this.slot = 3;
                case N4 -> this.slot = 4;
                case N5 -> this.slot = 5;
                case P -> board.printArray();
                case R -> ant.resetPos();
            }
        }
    }

    public Board getBoard() {
        return this.board;
    }

    public void toggleSimulation() {
        running = !running;
        if (running) {
            startSimulation();
        } else {
            stopSimulation();
        }
    }

    public void startSimulation() {
        running = true;
        System.out.println("start");
    }

    public void stopSimulation() {
        running = false;
        System.out.println("stop");
    }


}

Ant.java

    private Point2 pos;
    private char direction;
    Game game;
    Board board;

    public Ant(int x, int y, char direction, Game g) {
        this.game = g;
        this.board = game.getBoard();
        this.pos = new Point2(x, y);
        this.direction = direction;
    }

    public void resetPos() {
        this.pos.set(game.GRID_SIZE / 2, game.GRID_SIZE / 2); //somewhere in the middle of the grid, meh.
        this.direction = 'u';

    }

    //DIRECTIONS: pretty logical, chars for Up, Down, Left, Right.

    public Point2 getPos() {
        return this.pos;
    }

    public void moveForward() {
        board.updateTile(this.pos);
        int dx = switch (direction) {
            case 'u', 'd' -> 0;
            case 'l' -> -1;
            case 'r' -> 1;
            default -> throw new IllegalStateException("Unexpected value: " + direction);
        };
        int dy = switch (direction) {
            case 'r', 'l' -> 0;
            case 'u' -> -1;
            case 'd' -> 1;
            default -> throw new IllegalStateException("Unexpected value: " + direction);
        };

        this.pos.transform(dx, dy);

    }

    public void turnRight() {
        direction = switch (direction) {
            case 'u' -> 'r';
            case 'r' -> 'd';
            case 'd' -> 'l';
            case 'l' -> 'u';
            default -> throw new IllegalStateException("Unexpected value: " + direction);
        };
    }

    public void turnLeft() {
        direction = switch (direction) {
            case 'u' -> 'l';
            case 'l' -> 'd';
            case 'd' -> 'r';
            case 'r' -> 'u';
            default -> throw new IllegalStateException("Unexpected value: " + direction);
        };
    }

    public void turnBack() {
        direction = switch (direction) {
            case 'u' -> 'd';
            case 'l' -> 'r';
            case 'd' -> 'u';
            case 'r' -> 'l';
            default -> throw new IllegalStateException("Unexpected value: " + direction);
        };
    }
}    

Board.java

public class Board {

    Game game;
    int[][] boardArray;

    /*
    this is one of the classes which i used as a base, and modified.
    yeah dont have anything to add, pretty useful class with functions that are needed often.
     */

    public Board(Game g) {
        game = g;
        boardArray = new int[game.GRID_SIZE][game.GRID_SIZE];
        clearArray(boardArray);
    }

    public int getOnBoard(Point2 p) {
        return getOnBoard(p.getX(), p.getY());
    }

    public int getOnBoard(int x, int y) {
        if (isInBounds(x, y))
            return boardArray[x][y];

        return 0;
    }

    public void updateTile(Point2 p) {
        int var = getOnBoard(p);
        int newVar = var+1;
        if(newVar > 3){
            newVar = 0;
        }
        setOnBoard(p, newVar);
    }

    private void setOnBoard(Point2 p, int newVar) {
        setOnBoard(p.getX(), p.getY(), newVar);
    }

    public void setOnBoard(int x, int y, int v) {
        if (isInBounds(x, y))
            boardArray[x][y] = v;
    }

    public int countNonEmpty(int[][] arr) {
        int i = 0;
        for (int x = 0; x < game.GRID_SIZE; x++) {
            for (int y = 0; y < game.GRID_SIZE; y++) {
                if (arr[x][y] != 0) {
                    i++;
                }
            }
        }
        return i;
    }

    public void clearArray() {
        clearArray(boardArray);
    }

    public void clearArray(int[][] bArray) {
        if (bArray != null) {
            for (int x = 0; x < game.GRID_SIZE; x++) {
                for (int y = 0; y < game.GRID_SIZE; y++) {
                    bArray[x][y] = 0;
                }
            }
        }
    }

    public boolean isInBounds(Point2 p) {
        return isInBounds(p.getX(), p.getY());
    }

    public boolean isInBounds(int x, int y) {
        if (x >= 0 && x < game.GRID_SIZE && y >= 0 && y < game.GRID_SIZE) {
            return true;
        }
        //System.out.println("Out of Bounds!: " + x + " - " + y);
        return false;
    }

    public void printArray(int[][] array) {
        System.out.println("------------------------");
        for (int y = 0; y < game.GRID_SIZE; y++) {
            for (int x = 0; x < game.GRID_SIZE; x++) {
                System.out.print(array[x][y] + " ");
            }
            System.out.println();
        }
        System.out.println("------------------------\n\n\n\n");

    }

    public void printArray() {
        System.out.println("------------------------");
        printArray(boardArray);
    }

    public boolean contains(int v) {
        for (int x = 0; x < game.GRID_SIZE; x++) {
            for (int y = 0; y < game.GRID_SIZE; y++) {
                return boardArray[x][y] == v;
            }
        }
        return false;
    }

}

Loop.java

public class Loop {
    int stepsPerLoop = 1;
    boolean running = true;

    long startTime;
    int currentFrames;
    int fps;
    double stepDelay = 20;
    Game game;

    public Loop(Game g) {
        this.game = g;
    }

    public void run() {
        long lastloop = System.nanoTime();
        startTime = (lastloop / 1000000);
        long laststep = System.nanoTime();

        //my "gameloop". what do you think?
        while (running) {
            long now = System.nanoTime();

            if (game.running) {
                if ((now - laststep) / 1000000f >= stepDelay) {
                    for (int i = 0; i < stepsPerLoop; i++) {
                        game.step();

                    }
                    laststep = now;
                }
            }

            /*
            as you can see, i split the actual simulation and the rendering.
            this way, i can achieve higher simulation speeds while not having to render everytime.
             */

            if ((now - lastloop) / 1000000f >= 5) {
                game.frame.repaint();
                game.frame.setTitle("Sand  (fps: " + fps + ")  STEP: " + game.stepCnt + "  STEPDELAY: " + stepDelay + "  STEPSPERLOOP: " + stepsPerLoop);
                countFrame();
                lastloop = now;
            }
        }
    }

    public void countFrame() {
        long now = System.currentTimeMillis();
        if (now - startTime >= 1000) {
            startTime = now;
            fps = currentFrames;
            currentFrames = 0;

        }
        currentFrames += 1;
    }
}

Main.java

public class Main {

    /*
    pretty small main class.
     */

    public static void main(String[] args) {
        int gridSize = 600;  //width and height of the actual grid.
        int windowSize = 600;
        new Game(windowSize, gridSize);
    }
}

Point2.java

public class Point2 {
    private int x, y;

    /*
    simple 2d point class i made, so i dont have to pass 2 variables every time.
     */

    public Point2(int x, int y) {
        this.x = x;
        this.y = y;
    }

    public void set(int x, int y) {
        this.x = x;
        this.y = y;
    }

    public int getX() {
        return x;
    }

    public void setX(int x) {
        this.x = x;
    }

    public int getY() {
        return y;
    }

    public void setY(int y) {
        this.y = y;
    }

    public void transform(int dx, int dy) {
        this.x += dx;
        this.y += dy;
    }
}

I would've also added the GamePanel and GameFrame classes etc. but I feel like the question is already way too long. If it is not, I'm happy to also add those to the question. Thanks again.

Answer 1

CodeReview

General

In canonical java, it’s preferable to only have one variable declared per line, even if they share a type.

It is highly preferable for classes to not expose their member variables unless it is strictly required. Classes that cannot maintain internal invariants are at high risk of breaking because the users of the class can set it an invalid state either accidentally or maliciously. The overwhelming majority of non-private member variables in this project should be private. Making this modification will force consideration of what ideas belong to which classes, and how those classes manage those ideas. Note that right now all the major classes share themselves with each other.

Classes not designed for extension should disallow it by being marked as final.

For the sake of readability, code should avoid abbreviations. position is much more readable than pos, game is much better than g, etc.

Optional curly braces should always be included. They enhance readability and prevent a common type of error.

Member variables which will not change after assignment should be marked as final. This enhances readability and gives compiler protection against accidental assignment.

It’s confusing that some methods accept (x, y) or Point, while some only accept one or the other.

ALL_CAPS are reserved for constant values, that is, immutable static final member variables. Non-constant values should use camelCase.

Ant

An ant should not own the idea of a starting position. That concept belongs in another class. `Game` seems like a reasonable location for it, since the base position depends on variables owned by that class.

Direction should be a top-level enum, not a character. Enums are much more safe because they prevent invalid values, such as setting the direction to ‘q’.

Once Direction is its own enum, it makes sense for turnRight, turnLeft, and turnBack to belong to direction rather than Ant.

turnBack would be better named reverse(). It is also unused, and can be deleted.

The deltas for moveForward would also be better located in Direction.

Ant does not need to keep a copy of the game, since it is only used to find the board.

Board

`Game` is passed in, but only so that the GRID_SIZE can be read. Just pass in the GRID_SIZE directly and store it instead of the Game.

The clearArray call in the constructor does nothing, as int arrays are initialized with zeros already.

valueAt might be a better name than getOnBoard.

Returning zero when x, y are out of bounds should be clearly documented. I would expect an exception, not a zero. Silently failing to set a value when x, y are out of bounds should also be clearly documented, as I would again expect an exception. IllegalBoardPositionException, perhaps, or maybe PositionOutOfBoundsException.

setOnBoard might be better named setValueAt.

It’s unclear why the version of setOnBoard which takes a Point2 is private instead of public.

updateTile is the first and only mention that a board is composed of tiles. Perhaps updateValueAt would be better.

It’s unclear how updateTile is a generic concept that belongs to a board which is reused between games. If it is specific to this game, and Board is intended to be a library class, then this logic belongs in something specific to this particular game, and not the generic Board. Copying Board between projects is not an effective reuse strategy and should be avoided.

updateTile can be rewritten using a trinary operator.

countNonEmpty returns an int, but if the board is size Integer.MAX_INT, the number of values on the board will not fit in an int.

countNonEmpty can be rewritten to use streams. It may or may not be more clear than the current code to count.

countNonEmpty should not take in an int[][]. The board should own the int[][] and keep it private. countNonEmpty should not need a client to pass In what it already knows.

If they’re tiles, countNonEmpty might be better named countNonEmptyTiles.

clearArray should be just clear, or maybe reset. The fact that tan array is used to store the board is an implementation detail that clients should neither know nor care about.

clearArray should not accept an int[][] for the same reasons as above.

clearArray can be simplified by using Arrays.fill.

isInBounds can be simplified to return the result of the boolean computation rather than branching on it.

printArray should not accept an int[][] for the same reasons as above. Board should definitely not be printing random int arrays.

Rather than printing directly from Board to console, Board should implement toString. The contents of printArray should move to toString, which would return a string representation of the board. The caller of toString could then print it, write it to a file, or do whatever else it wanted to.

printArray currently assumes a gridSize > 0. This is not unreasonable, but it needs to be tested and validated.

contains might be better named containsValue.

contains can also use a stream. Since it’s in two places, the stream generation code can be extracted to a private method.

Game

The gameFrame and stepCount should be made available by accessor, not with public variables.

mouseX and mouseY are local to onAction, and should be rescoped to there.

WINDOW_SIZE is used only in the constructor and should not be a member variable.

With an immutable Point2, one default ant position can be stored rather than creating a new one every time. This isolates GRID_SIZE to the constructor, so it does not need to be a member variable.

Maintaining a static variable in Game that tracks the most recently created game does not make much sense.

Running the game loop on the same thread that the game is constructed on, in the constructor for the game, is concerning. It means the game is never fully instantiated until “Loop” is done running. The looping code should be a separate thread spawned off from the Game constructor.

drawOnBoard should have a default case to trap any other action values.

L_MOUSE is not an action. It’s a user input. DRAW_ON_BOARD is an action. Likewise all the other Actions.

The switch for L_MOUSE can be simplified to board.setValueAt(x, y, slot)

If the code needs to log, it should use a logger, not system.out.

The MouseWheelEvent appears to do nothing and can be removed.

One big method to handle every possible input type is not an extensible design. The input-gathering code does not appear to be under review, but strongly consider a different approach here. Perhaps the Command design pattern would be appropriate here. There might need to be two command interfaces, one that includes a position for mouse clicks and one that does not for key presses.

The big switch method needs a default -> {} case.

startSimulation and stopSimulation appear to be only called by toggleSimulation. They should be folded into that method.

drawOnBoard and toggleSimulation are all only accessed from this class and can be made private.

Main

It’s odd that windowSize and gridSize are declared out of the order that they’re used.

If windowSize and gridSize are constants, they should be declared as private static final int at a class level.

Point2

transform() does not account for overflow or underflow. For instance, if x equals Integer.MAX_INT, and it gets transformed by +5, the user of the class will be rather surprised. Test for, mitigate, and document the behavior of overflow and underflow. `Math.addExact` will throw an exception for overflow or underflow.

Immutable classes have numerous benefits and few drawbacks. Strongly consider making this class immutable.

Consider a static factory. Point2.of(x, y) is arguably easier to read than new Point2(x, y).

Loop

stepsPerLoop and stepDelay should have accessors rather than being directly exposed.

There’s more to say about Loop, but I’m tired. I will point out that a lot of information stored in Game is used primarily in Loop, and vice versa. Strongly consider if you’ve identified the correct boundary between the two classes.