Explain Conway's game of life Java.
I have just finished implementing a version of Conway's Game of Life using Java.
Being only a college student, I am sure that my code is nowhere near perfect, and was wondering if you could look at my code. What can I improve on? Are there faster ways to implement certain areas of my code? Is there excess code that I can trim away? Is there a smarter way of implementing Conway's Game of Life?
First of all, I think the algorithm is pretty smart which is, for my humble experience, not so common for a college student. So congrats if you came up with it by yourself! If you're looking for smart implementations I would recommend functional ones, e.g. in Haskell; see also Shortest game of life. Now, beware of smartness. A good code should be easy to read, easy to understand. This is of course not always possible when dealing with complex algorithms but I believe that it should be a target.
jjjjjjjjjjjj said: Note: most comments have been taken out because they don't add much to the readability of the code
The point of comments is to help people understand your code (generally speaking, focus on the "why" rather than on the "what"). Here, to help people understand you had to add a lot of text to your post. Ideally this isn't needed because the code is:
self-documented,
commented to clear complex/implicit stuff up.
For instance, here is a quick rewrite of your code in an attempt to make the code more expressive:
Game Of Life java
/**
* Computes the next state of the automaton by using Conway's original rules.
*/
public class GameOfLife extends CellularAutomaton {
/**
* Stores all cells in a two-dimensional matrix. The value stored is
* the number of live neighbours of the cell, +10 if the cell is alive.
*/
private int board[][];
private int dim;
/*
* index(cell) = cellX * dim + cellY
*/
private Stack indexesOfCellsAliveAtNextGeneration;
private HashMap cellsMaybeAliveAtNextGeneration;
public GameOfLife(int d, Stack s){
board = new int[d][d];
dim = d;
indexesOfCellsAliveAtNextGeneration = s;
cellsMaybeAliveAtNextGeneration = new HashMap<>();
}
public String newGeneration() {
populateWorldWithAliveCellsFromPreviousGeneration();
computeCellsMaybeAliveAtNextGeneration();
return boardAsString();
}
private void populateWorldWithAliveCellsFromPreviousGeneration() {
for (Map.Entry cell : cellsMaybeAliveAtNextGeneration.entrySet()) {
int cellIndex = cell.getKey();
int cellValue = cell.getValue();
if(willBeAlive(cellValue)){
indexesOfCellsAliveAtNextGeneration.add(cellIndex);
}
board[cellIndex/dim][cellIndex%dim] = 0;
}
}
private static boolean willBeAlive(int cell){
return (!isAlive(cell) && nbOfNeighbors(cell) == 3)
|| (isAlive(cell) && (nbOfNeighbors(cell) == 2 || nbOfNeighbors(cell) == 3));
}
private static boolean isAlive(int cell) {
return cell >= 10;
}
private static int nbOfNeighbors(int cell) {
return cell ;
}
private void computeCellsMaybeAliveAtNextGeneration() {
cellsMaybeAliveAtNextGeneration.clear();
while(!indexesOfCellsAliveAtNextGeneration.isEmpty()) {
int cellIndex = indexesOfCellsAliveAtNextGeneration.pop();
int cellX = cellIndex / dim;
int cellY = cellIndex % dim;
int topLeftNeighbourX = (cellX <= 0) ? 0 : cellX - 1;
int topLeftNeighbourY = (cellY <= 0) ? 0 : cellY - 1;
int bottomRightNeighbourX = (cellX >= dim - 1) ? cellX + 1 : cellX + 2;
int bottomRightNeighbourY = (cellY >= dim - 1) ? cellY + 1 : cellY + 2;
// Iterate through every cell's neighbour to increase their neighbour number
for(int i = topLeftNeighbourX; i < bottomRightNeighbourX xss=removed xss=removed xss=removed xss=removed xss=removed xss=removed xss=removed>I mostly renamed some variables/methods and introduced some utility methods. The code is a bit longer and feels more verbose but is IMHO also easier to understand. It is still very procedural (which is not bad per se, especially for such a simple program) but you may want to try to add more expressiveness by introducing new classes such as Board or Cell. You'll find such OO implementations on GitHub.
Your code may also run into memory issues with large boards. Indeed, your board[][] variable stores all the cells, even dead ones. With a 10000 x 10000 board containing only ~5/6 cells you'll waste a lot of memory. A solution is to use a sparse array (basically, a set containing only alive cells).
As a side note, a few years ago I also tried to model a highly-configurable GoL in a "pure" OO way; my code is on GitHub if you want to check it out. The method computing the next generation of the world is ImmutableGeneration::nextGeneration; given a set of alive cells, it basically: 1) compute all neighbours cells then 2) keep only those that will be alive. Rules indicating whether a cell will be alive or dead are implemented in Rule.java.