3
\$\begingroup\$

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);
 }
}
200_success
146k22 gold badges190 silver badges479 bronze badges
asked Feb 25, 2018 at 13:22
\$\endgroup\$

1 Answer 1

-1
\$\begingroup\$

In equals a cast to Cellis 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));
}
answered Feb 25, 2018 at 20:27
\$\endgroup\$
3
  • 1
    \$\begingroup\$ Did you mean if (not o instanceof Cell) return false;? \$\endgroup\$ Commented 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 the if 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\$ Commented Feb 26, 2018 at 8:00
  • 1
    \$\begingroup\$ Don't you need the not? \$\endgroup\$ Commented Feb 26, 2018 at 13:01

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.