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);
    }
}
\$\endgroup\$
-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));
}
\$\endgroup\$
  • 1
    \$\begingroup\$ Did you mean if (not o instanceof Cell) return false;? \$\endgroup\$ – Solomon Ucko Feb 25 '18 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\$ – Imus Feb 26 '18 at 8:00
  • 1
    \$\begingroup\$ Don't you need the not? \$\endgroup\$ – Solomon Ucko Feb 26 '18 at 13:01

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.