I've recently had a go at programming a Conway's Game of Life simulation in Java, and I was wondering if anyone could provide some input on how it could be improved. I'm mainly concerned with how to improve the "cleanliness" and readability of the code, rather than it's algorithmic complexity. This is my first attempt at using Java (although I used other OOP languages) so if I'm not using any obvious Java functionality please let me know!
The Game
Class:
package com.company;
import java.util.HashSet;
import java.util.Set;
public class Game {
private Set<Cell> cells = new HashSet<>();
public Game() {}
public void addCell(Cell cell) {
cells.add(cell);
}
public boolean hasAliveCellAt(Cell cell) {
return cells.contains(cell);
}
public int getNumberOfAliveCells() {
return cells.size();
}
public void tick() {
Set<Cell> nextTicksCells = new HashSet<>();
nextTicksCells.addAll(getCellsThatSurviveTick());
nextTicksCells.addAll(getCellsThatAreBornFromTick());
cells = nextTicksCells;
}
private Set<Cell> getCellsThatSurviveTick() {
Set<Cell> survivingCells = new HashSet<>();
for (Cell cell : cells) {
if (cellCanSurvive(cell)) {
survivingCells.add(cell);
}
}
return survivingCells;
}
private boolean cellCanSurvive(Cell cell) {
int numberOfAliveNeighbours = getNumberOfAliveNeighbours(cell);
return cellHasCorrectAmountOfNeighboursToSurvive(numberOfAliveNeighbours);
}
private int getNumberOfAliveNeighbours(Cell cell) {
int numberOfAliveNeighbours = 0;
for (int x = (cell.x - 1); x <= (cell.x + 1); x++) {
for (int y = (cell.y - 1); y <= (cell.y + 1); y++) {
Cell neighbouringCell = new Cell(x, y);
if (cell.equals(neighbouringCell)) {
continue;
}
if (hasAliveCellAt(neighbouringCell)) {
numberOfAliveNeighbours++;
}
}
}
return numberOfAliveNeighbours;
}
private boolean cellHasCorrectAmountOfNeighboursToSurvive(int numberOfAliveNeighbours) {
return (numberOfAliveNeighbours == 2) || (numberOfAliveNeighbours == 3);
}
private Set<Cell> getCellsThatAreBornFromTick() {
Set<Cell> deadNeighbouringCells = getGamesDeadNeighbouringCells();
Set<Cell> cellsToBeBorn = new HashSet<>();
for (Cell cell : deadNeighbouringCells) {
if (cellCanBeBorn(cell)) {
cellsToBeBorn.add(cell);
}
}
return cellsToBeBorn;
}
private Set<Cell> getGamesDeadNeighbouringCells() {
Set<Cell> deadNeighbouringCells = new HashSet<>();
for (Cell cell : cells) {
deadNeighbouringCells.addAll(getCellsDeadNeighbouringCells(cell));
}
return deadNeighbouringCells;
}
private Set<Cell> getCellsDeadNeighbouringCells(Cell cell) {
Set<Cell> deadNeighbouringCells = new HashSet<>();
for (int x = (cell.x - 1); x <= (cell.x + 1); x++) {
for (int y = (cell.y - 1); y <= (cell.y + 1); y++) {
Cell neighbouringCells = new Cell(x, y);
if (cell.equals(neighbouringCells)) {
continue;
}
if (!hasAliveCellAt(neighbouringCells)) {
deadNeighbouringCells.add(neighbouringCells);
}
}
}
return deadNeighbouringCells;
}
private boolean cellCanBeBorn(Cell cell) {
int numberOfAliveNeighbours = getNumberOfAliveNeighbours(cell);
return cellHasCorrectAmountOfNeighboursToBeBorn(numberOfAliveNeighbours);
}
private boolean cellHasCorrectAmountOfNeighboursToBeBorn(int numberOfAliveNeighbours) {
return (numberOfAliveNeighbours == 3);
}
}
The Cell
Class:
package com.company;
import java.util.Objects;
public class Cell {
public final int x;
public final int y;
public Cell(int x, int y) {
this.x = x;
this.y = y;
}
@Override
public boolean equals(Object o) {
if (this == o) return true;
if (o == null || getClass() != o.getClass()) return false;
Cell cell = (Cell) o;
return ((x == cell.x) && (y == cell.y));
}
@Override
public int hashCode() {
return Objects.hash(x, y);
}
}
1 Answer 1
In equals
a cast to Cell
is required. Before doing the cast, the type of the argument needs to be checked by the instanceof
Operator to avoid a ClassCastException
. As instanceof
returns false
if the first oerand is null
the check can be simplfied:
@Override public boolean equals(Object o) {
if (this == o) return true;
if (!(o instanceof Cell)) return false;
Cell cell = (Cell) o;
return ((x == cell.x) && (y == cell.y));
}
-
1\$\begingroup\$ Did you mean
if (not o instanceof Cell) return false;
? \$\endgroup\$Solomon Ucko– Solomon Ucko2018年02月25日 23:06:07 +00:00Commented Feb 25, 2018 at 23:06 -
\$\begingroup\$ His current implementation is better in case
Cell
can also have subclasses that don't really need to override the equals method. Seeing that it's also the only method where he's not using{ }
after theif
statement I'm assuming he let his IDE generate the equals (and hashcode) which is often a better idea than to try to implement it yourself. \$\endgroup\$Imus– Imus2018年02月26日 08:00:10 +00:00Commented Feb 26, 2018 at 8:00 -
1\$\begingroup\$ Don't you need the
not
? \$\endgroup\$Solomon Ucko– Solomon Ucko2018年02月26日 13:01:53 +00:00Commented Feb 26, 2018 at 13:01