Coded the Game of Life

Question

I am a beginner to java and I've coded the Game of Life! For those not familiar, the game entails creating a grid of specified dimensions with each box in the grid being either dead or alive. The grid starts with a random dead or alive state for each box. New generations have new dead/alive boxes based on the following conditions: If there are 3 alive boxes neighbouring any dead box, it becomes alive. If there are less than 2 alive boxes neighbouring an alive box, that box dies. It also dies if there are more than 3 alive boxes around it.

The way I am approaching this is by making a boolean[][] array grid with true representing an alive box and false representing a dead one. The view class takes this boolean grid and turns it into a grid of boxes that are either coloured white or red (white = dead)(red = alive).

Game Class:

import java.util.Arrays;
import java.util.Random;

public class Game {
    private boolean[][] grid;
    private int genCount;
    private int width;
    private int height;

    public Game(int i, int j) {
        genCount = 0;
        if (i < 1 || j < 1) {
            throw new IllegalArgumentException("grid too small!");
        }
        if (i > 100 || j > 100) {
            throw new IllegalArgumentException("grid too big!");
        }
        width = i;
        height = j;
        grid = new boolean[j][i];
    }

    public int getWidth() {
        return width;
    }
    public int getHeight() {
        return height;
    }

    public boolean[][] getGrid() {
        return grid;
    }

    public void setGrid(boolean[][] grid) {
        this.grid = grid;
    }

    public int getGen() {
        return genCount;
    }

    public void setGen(int gen) {
        this.genCount = gen;
    }

    public void randomGrid(double probability) {
        Random r = new Random();
        for (int j = 0; j < height; j++) {
            for (int i = 0; i < width; i++) {
                grid[j][i] = Math.random() < probability;
            }
        }
    }

    public boolean[][] makeCopy() {
        // making a copy of the current grid, avoiding aliasing.
        boolean[][] nGrid = new boolean[height][width];
        for (int j = 0; j < height; j++) {
            for (int i = 0; i < width; i++) {
                nGrid[j][i] = grid[j][i];
            }
        }
        return nGrid;
    }

    public void newGen() {

        boolean[][] nGrid = makeCopy();

        for (int j = 0; j < height; j++) {
            for (int i = 0; i < width; i++) {
                if (grid[j][i] == false) {
                    // new life, due to having 3 neighbours!
                    if (getNeighbours(j, i) == 3) {
                        nGrid[j][i] = true;
                    }
                } else {
                    // isolation death!
                    if (getNeighbours(j, i) <= 1) {
                        nGrid[j][i] = false;
                    }
                    // overcrowing death!
                    if (getNeighbours(j, i) >= 4) {
                        nGrid[j][i] = false;
                    }
                }
            }
        }
        genCount++;
        grid = nGrid;
    }

    public int getNeighbours(int j, int i) {
        int count = 0;
        int jMax = grid[0].length - 1;
        int iMax = grid[1].length - 1;

        // checking up, down, right, left, allowing wrapping
        if ((j < jMax && grid[j + 1][i]) || (j == jMax && grid[0][i])) {
            count++;
        }
        if ((j > 0 && grid[j - 1][i]) || (j == 0 && grid[jMax][i])) {
            count++;
        }
        if ((i < iMax && grid[j][i + 1]) || (i == iMax & grid[j][0])) {
            count++;
        }
        if ((i > 0 && grid[j][i - 1]) || (i == 0 && grid[j][iMax])) {
            count++;
        }

        // checking diagonals, allowing wrapping
        if ((j < jMax && i < iMax && grid[j + 1][i + 1]) || (j == jMax && i < iMax && grid[0][i + 1])
                || (j < jMax && i == iMax && grid[j + 1][0]) || (j == jMax && i == iMax && grid[0][0])) {
            count++;
        }
        if ((j < jMax && i > 0 && grid[j + 1][i - 1]) || (j == jMax && i > 0 && grid[0][i - 1])
                || (j < jMax && i == 0 && grid[j + 1][iMax]) || (j == jMax && i == 0 && grid[0][iMax])) {
            count++;
        }
        if ((j > 0 && i < iMax && grid[j - 1][i + 1]) || (j == 0 && i < iMax && grid[jMax][i + 1])
                || (j > 0 && i == iMax && grid[j - 1][0]) || (j == 0 && i == iMax && grid[jMax][0])) {
            count++;
        }
        if ((j > 0 && i > 0 && grid[j - 1][i - 1]) || (j == 0 && i > 0 && grid[jMax][i - 1])
                || (j > 0 && i == 0 && grid[j - 1][iMax]) || (j == 0 && i == 0 && grid[jMax][iMax])) {
            count++;
        }
        return count;
    }

}

View Class:

import java.awt.Color;
import java.awt.Graphics;
import javax.swing.JFrame;
import javax.swing.JPanel;

public class View extends JPanel {

    public void paint(Graphics g) {

        Game game = new Game(10, 10);
        game.randomGrid(0.2);
        game.newGen();
        boolean[][] grid = game.getGrid();

        g.setColor(Color.red);
        for (int j = 0; j < game.getHeight(); j++) {
            for (int i = 0; i < game.getWidth(); i++) {
                g.drawRect(100 + (50 * i), 100 + (50 * j), 50, 50);
                if (grid[j][i]) {
                    g.fillRect(100 + (50 * i), 100 + (50 * j), 50, 50);
                }
            }
        }

    }

    public static void main(String[] args) {
        JFrame frame = new JFrame();
        frame.getContentPane().add(new View());

        frame.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
        frame.setSize(1000, 1000);
        frame.setVisible(true);
    }
}

Answer 1

You are not assigning default values to the fields and you are splitting game logic and UI. That's a good start.

public Game(int i, int j) {

In a public function or constructor I expect clearly named parameters.

if (i > 100 || j > 100) {
    throw new IllegalArgumentException("grid too big!");
}

So a grid of 101 cells is too big if i = 1 and j = 101 but a field of 10 thousand cells is not too big if i = 100 and j = 100? I'd return specific errors for the width and height parameters.

width = i;

Generally we simply use this.width = width instead, so we don't have to come up with a different variable name.

public boolean[][] getGrid() {
    return grid;
}

Beware of exposing your state. The reference to the grid is copied, but the arrays are mutable! Either clone or expose the state in another way (e.g. using an enumerator or by referencing grid cells separately using coordinates).

public void setGen(int gen) {
    this.genCount = gen;
}

It's a bit weird that you can set the genCount during the game of life. Again, setGen and gen may not really be clear to the user. gen may also have any value apparently, allowing the object instance to have an invalid state. I'd rename genCount to generation, so away with gen altogether and remove this setter.

public void randomGrid(double probability) {

This is OK-ish, but I prefer to check if probabilities are between 0 and 1 inclusive (yeah, I know, Java is rather heavy on boilerplate code).

public boolean[][] makeCopy() {

This seems near identical to getGrid. But more importantly, if I make a copy of a game then I don't expect a grid in return. This method is in dire need to be made private anyway.

boolean[][] nGrid = new boolean[height][width];

Just newGrid is only 2 characters extra. Don't overly skim on characters (and possibly learn how to touch type).

for (int i = 0; i < width; i++) {
    nGrid[j][i] = grid[j][i];
}

Use System.arrayCopy() at least for the inner loop instead.

if (grid[j][i] == false) {

Ugh, here are the i and j again, but now j is the X-coordinate while it was the width before and i is the Y-coordinate.

Furthermore, what about two constants? DEAD = false and ALIVE = true? Using an enum is also possible (enum Cell { DEAD, ALIVE; }), so that false and true cannot even be used anymore.

if (getNeighbours(j, i) == 3) {

This I like, good method name, and a method that is sorely needed here.

genCount++;
grid = nGrid;

Lovely, side effects right at the end where they belong.

---

As for getNeighbours, let's simplify things a bit using modular arithmetic...

private static final boolean ALIVE = true;

private int getNeighbours(int x, int y) {
    int count = 0;

    // iterate over all neighbouring cells as a square
    for (int dx = -1; dx <= 1; dx++) {
        for (int dy = -1; dy <= 1; dy++) {
            // skip cell itself
            if (dx == 0 && dy == 0) {
                continue;
            }

            // get coordinates for neighbour
            int nx = Math.floorMod(x + dx, width);
            int ny = Math.floorMod(y + dy, height);

            if (grid[nx][ny] == ALIVE) {
                count++;
            }
        }
    }

    return count;
}

Note that floorMod is the modulus operation while % is the remainder operation which will return -1 if the left operand is -1. That's not what you want, you want width - 1 or height - 1 instead.

Now I hope that looks a bit easier on the eye. You may want to rename dx to deltaX and nx to neighbouringX or something like that, but there is such a thing as overdoing it too. For local vars you may be a bit less strict (fields and parameters are more important for sure).

Note that I didn't know that the game of life uses wrapping, but that's another matter I guess. I'll not go on about the strategy design pattern, that might be a bit too deep.

---

I think your design would look better if you split your game of life into a Game and Grid class. That way the method naming would be much easier.

---

Finally, currently your game of life only operates when redrawing the window (or JFrame in Swing talk). That's of course not how it should be, it should be running on a timer and/or using buttons.

Answer 2

I made some modifications to your View class and created this GUI.

I couldn't get the Game class to cycle through generations. Maybe I missed something. I didn't see where the Game class needed any changes.

Here are the changes I made to your View class.

1. I started the GUI on the Event Dispatch Thread (EDT) by calling the SwingUtilities invokeLater method. This ensures that the Swing components are created and updated on the EDT.

2. I put the painting code in the paintComponent method. I called super.paintComponent to maintain the Swing paint chain. I removed all code that wasn't painting code.

3. I put @Override annotations on all of the methods I overrode. That way, the compiler will possibly tell me if there's a problem with my method calls.

4. I scaled the JPanel to a more reasonable size. Some of us still have tiny monitors. I got rid of most of the magic numbers.

Here's the View class code.

import java.awt.Color;
import java.awt.Dimension;
import java.awt.Graphics;

import javax.swing.JFrame;
import javax.swing.JPanel;
import javax.swing.SwingUtilities;

public class View extends JPanel implements Runnable {

    private static final long serialVersionUID = 1L;

    private boolean[][] grid;

    private int margin;
    private int squareWidth;

    private Game game;

    public View() {
        this.game = new Game(10, 10);
        this.game.randomGrid(0.2);
        this.grid = game.getGrid();

        this.margin = 50;
        this.squareWidth = 32;
        int width = squareWidth * 10 + margin + margin;
        this.setPreferredSize(new Dimension(width, width));
    }

    @Override
    public void run() {
        JFrame frame = new JFrame("Conway's Game of Life");
        frame.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);

        frame.getContentPane().add(new View());
        frame.pack();
        frame.setLocationByPlatform(true);
        frame.setVisible(true);

        Thread thread = new Thread(new Timer(game, this));
        thread.start();
    }   

    public void setGrid(boolean[][] grid) {
        this.grid = grid;
    }

    @Override
    protected void paintComponent(Graphics g) {
        super.paintComponent(g);

        int generation = game.getGen();
        String text = "Generation: " + generation;
        g.setColor(Color.BLACK);
        g.drawString(text, 10, 30);

        g.setColor(Color.red);
        for (int j = 0; j < grid.length; j++) {
            for (int i = 0; i < grid[j].length; i++) {
                int x = margin + (squareWidth * i);
                int y = margin + (squareWidth * j);
                g.drawRect(x, y, squareWidth, squareWidth);
                if (grid[j][i]) {
                    g.fillRect(x, y, squareWidth, squareWidth);
                }
            }
        }

    }

    public static void main(String[] args) {
        SwingUtilities.invokeLater(new View());
    }

} 

I created a new Timer class to update the GUI every 5 seconds. Since the GUI never changed, I put a System.out.println method in the timer loop to make sure it was running.

You can change the 5 second delay if you wish.

import javax.swing.SwingUtilities;

public class Timer implements Runnable {

    private volatile boolean running;

    private Game game;

    private View view;

    public Timer(Game game, View view) {
        this.game = game;
        this.view = view;
        this.running = true;
    }

    @Override
    public void run() {
        while (running) {
            sleep(5000L);
            game.newGen();
            updateView();
//          System.out.println("running");
        }
    }

    private void sleep(long duration) {
        try {
            Thread.sleep(duration);
        } catch (InterruptedException e) {
            // Deliberately left empty
        }
    }

    private void updateView() {
        SwingUtilities.invokeLater(new Runnable() {
            @Override
            public void run() {
                view.setGrid(game.getGrid());
                view.repaint();
            }
        });
    }

    public synchronized void setRunning(boolean running) {
        this.running = running;
    }

}

Answer 3

I would recommend changing how your grid is represented in memory. Imagine you have a 10000x10000 grid and just 1 cell at coordinates (5,5) that's alive.

If you use an array to store that information, you have to create thousands upon thousands of data points pretty much all of which basically say nothing but “nothing to see here”. Dead cells are the default and therefore there's not really much we need to hold in memory about them.

So let's use a HashSet that only stores the alive cells. If the HashSet contains a given coordinate, the corresponding cell is alive. Otherwise the HashSet returns false, meaning it is dead. Now you can have a virtually infinite grid which, especially if mostly empty (as a Game of Life usually is), hardly needs any memory.

My Java is super rusty so consider the following something like pseudo code, but you get the idea:

public class Coordinate {
    public int x, y;

// Just to make it even clearer than I thought it was: 
// There is more work to do in this code snippet, like overriding equals() and hashCode(). 
// This code is just to demonstrate the concept, not an actual full implementation.

}

public class Grid {
    private HashSet grid;

    public void setAlive(Coordinate c) {
        grid.add(c);
    }

    public void setDead(Coordinate c) {
        grid.remove(c);
    }

    public boolean isAlive(Coordinate c){
        return grid.contains(c);
    }

    public int getNumberOfNeighbours(Coordinate c){
        ...
    }
}

Obviously you can now continue to encapsulate other grid-specific functions and make your Game class simpler and better readable.

Answer 4

I can't speak to the Java-related aspects of your solution, but you've got one problem that would be an issue in any language: you're performing far more dynamic allocation than necessary.

In your case, you know exactly how much space you need: two grids of width*height space each. Instead of allocating a new grid every generation, just allocate the space you need at game startup. With each generation, swap which one represents the current state, and which one represents the next state. You can do this by allocating grid as a three-dimensional array, or by allocating two grids and swapping references, or whatever other method you think is easiest.

Answer 5

A few quick notes on newGen(). The main one is you're calling getNeighbours twice in the cell-was-alive case, meaning you're doing double the work to count them when the value can't change. So you should only call it once and save the result in a variable, and since you're going to need the count in all cases so you can do it once before the if:

    for (int j = 0; j < height; j++) {
        for (int i = 0; i < width; i++) {
            int neighbours = getNeighbours(j, i);

I'd then continue with the 'alive' case since it's the 'true' case, but that doesn't really matter. Sticking with what you had, this becomes

            if (grid[j][i] == false) {
                // new life, due to having 3 neighbours!
                if (neighbours == 3) {
                    nGrid[j][i] = true;
                }
            } else {
                // isolation death!
                if (neighbours <= 1) {
                    nGrid[j][i] = false;
                }
                // overcrowing death!
                if (neighbours >= 4) {
                    nGrid[j][i] = false;
                }
            }

But as it stands you're copying the grid and then setting or resetting it for changes whereas we do explicitly know whether each cell is dead or alive in the conditions. If we change this to:

            if (grid[j][i] == false) {
                // new life, due to having 3 neighbours!
                if (neighbours == 3) {
                    nGrid[j][i] = true;
                } else {
                    // still dead
                    nGrid[j][i] = false;
                }
            } else {
                // isolation death!
                if (neighbours <= 1) {
                    nGrid[j][i] = false;
                } else if (neighbours >= 4) {
                    // overcrowing death!
                    nGrid[j][i] = false;
                } else {
                    // still alive
                    nGrid[j][i] = true;
                }
            }

then we're always setting the cell in nGrid, meaning you no longer need to copy the old grid: you can just allocate an empty array instead.

    // in newGen(), replacing the call to makeCopy()
    boolean[][] nGrid = new boolean[height][width];

Or we could simplify this condition further:

            if (grid[j][i] == false) {
                // new life if 3 neighbours, else dead
                nGrid[j][i] = (neighbours == 3);
            } else {
                // still alive if 2 or 3 neighbours, else dead
                nGrid[j][i] = (neighbours == 2) || (neighbours == 3);
            }

or even flipping the logic slightly:

            // New cell is alive if it has three neighbours, regardless whether it was
            // dead or alive before, or if it was already alive and has two neighbours.
            nGrid[j][i] = (neighbours == 3) || (grid[j][i] && (neighbours == 2));

although I appreciate the previous way is a clearer statement of the rules.