• 29.5k
• 16
• 45
• 202

Does this look like bad design? Conways Game of Life in Java

• 61
• 1
• 4

I created a program in Java that showcases Conway's Game of Life, in 4 days. Created the algorithm from scratch. However I got a scathing review and left me scratching my head. This was for a job application, the guy didn't say anything more, other than: "Kevin – we are going to pass. He has good documentation and coding hygiene, but his algorithms and design choices are questionable. It really does not make sense based on his background." I submitted it to Codacity and got a B based on style and such, I mean given the short time I had, I chose having more features, than elegant code. Could someone help me to understand what I did that was so bad.

You can see the whole bit here: https://github.com/kevingnet/GameOfLife/tree/master/GameOfLife

/* (non-Javadoc)
 * @see Cells#step()
 * Calculate next generation of cells
 * Algorithm:
 * - get both cells from the two buffers, both have the exact same location
 * - count live neighbors, regardless of state, somewhat optimal depending on state
 * - apply game rules: liveCell (2 or 3 liveNeighbors) = live, deadCell (3 liveNeighbors) = live
 * - clear cell buffer, by killing all cells
 * - swap buffers, readying for next step
 */
@Override
public void step() {
 for (int i = 0; i < this.cells.size(); ++i) {
 Cell cp = this.cells.get(i);
 Cell bcp = this.cellsBuffer.get(i);
 // count neighbor states, location is cached as adjacent cells contain references
 int liveNeighborsCount = getNeighborCount(cp);
 if (cp.isAlive()) {
 if (liveNeighborsCount == 2 || liveNeighborsCount == 3) {
 bcp.revive();
 }
 } else if (cp.isDead()) {
 if (liveNeighborsCount == 3) {
 bcp.revive();
 }
 }
 }
 // clear grid = kill all cells
 // we don't kill them before, since it would cause neighbor miscalculations
 for (Cell cp : this.cells) {
 cp.kill();
 }
 // swap arrays for next iteration
 ArrayList<Cell> tmp = this.cellsBuffer;
 this.cellsBuffer = this.cells;
 this.cells = tmp;
}
/**
 * Recalculate engine cell grid
 * use double buffer, so as to avoid unnecessary memory allocation and deallocation
 * Algorithm:
 * - save current alive cells into array
 * - resize cell buffer and add cells set to dead initially
 * - for each cell save neighbor locations
 * - clone array, both contain only dead cells
 * - copy saved alive cells into cell array
 * - save current dimensions
 * - restored saved cells into resized array
 */
private void recalculate() {
 // save alive cells
 ArrayList<Cell> alive = new ArrayList<>(0);
 for (Cell nc : this.cells) {
 if (nc.isAlive()) {
 alive.add(nc);
 }
 }
 int c = this.dimensions.width;
 int r = this.dimensions.height;
 this.cellsBuffer = new ArrayList<>(c * r);
 // initialize array, identity is set to dead, so first step is recalculated
 Cell cp = null;
 Point p = null;
 for (int i = 0; i < r; ++i) {
 for (int j = 0; j < c; ++j) {
 p = new Point(i, j);
 cp = new Cell(p);
 this.cellsBuffer.add(cp);
 }
 }
 // calculate and set neighbors
 if (this.teleport) {
 calculateNeighborsStiched();
 } else {
 calculateNeighbors();
 }
 
 //clone cell array
 this.cells = new ArrayList<>(c * r);
 for (Cell bcp : this.cellsBuffer) {
 this.cells.add(new Cell(bcp));
 }
 this.columns = c;
 this.rows = r;
 // copy old to current
 for (Cell ocp : alive) {
 Point op = ocp.getLocation();
 cp = this.cells.get((op.x * this.columns) + op.y);
 cp.revive();
 }
}
//cells are confined to the grid without teleporting capability
private void calculateNeighbors() {
 int c = this.dimensions.width;
 int r = this.dimensions.height;
 int col = 0;
 int row = 0;
 for (Cell ncp : this.cellsBuffer) {
 Point np = ncp.getLocation();
 int i = np.x;
 int j = np.y;
 ArrayList<Point> neighbors = new ArrayList<>(8);
 // go around the cell...
 // top
 row = i - 1;
 col = j;
 if (row >= 0) {
 addNeighbor(ncp, row, col, c, neighbors);
 }
 // bottom
 row = i + 1;
 col = j;
 if (row < r) {
 addNeighbor(ncp, row, col, c, neighbors);
 }
 // top left
 row = i - 1;
 col = j - 1;
 if (col >= 0 && row >= 0) {
 addNeighbor(ncp, row, col, c, neighbors);
 }
 // top right
 row = i - 1;
 col = j + 1;
 if (col < c && row >= 0) {
 addNeighbor(ncp, row, col, c, neighbors);
 }
 // bottom left
 row = i + 1;
 col = j - 1;
 if (col >= 0 && row < r) {
 addNeighbor(ncp, row, col, c, neighbors);
 }
 // bottom right
 row = i + 1;
 col = j + 1;
 if (col < c && row < r) {
 addNeighbor(ncp, row, col, c, neighbors);
 }
 // left
 row = i;
 col = j - 1;
 if (col >= 0) {
 addNeighbor(ncp, row, col, c, neighbors);
 }
 // right
 row = i;
 col = j + 1;
 if (col < c) {
 addNeighbor(ncp, row, col, c, neighbors);
 }
 ncp.setNeighbors(neighbors);
 }
}