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
/* (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);
}
}