4
\$\begingroup\$

I am currently only doing one generation. I'd like a review of my code.

Game of Life: The universe of the Game of Life is an infinite two-dimensional orthogonal grid of square cells, each of which is in one of two possible states, live or dead. Every cell interacts with its eight neighbours, which are the cells that are directly horizontally, vertically, or diagonally adjacent. At each step in time, the following transitions occur:

  1. Any live cell with fewer than two live neighbours dies, as if by loneliness.
  2. Any live cell with more than three live neighbours dies, as if by overcrowding.
  3. Any live cell with two or three live neighbours lives, unchanged, to the next generation.
  4. Any dead cell with exactly three live neighbours comes to life.

Game.java:

package tw51.assignment.gameoflife;
import java.util.*;

import static tw51.assignment.gameoflife.Cells.State;
import static tw51.assignment.gameoflife.Cells.State.Alive;
import static tw51.assignment.gameoflife.Cells.State.Dead;

/**
 * Represents A Game with a set Of rules that forms patterns recursively.
 */
public class Game {


    private List<Cells> aliveCells = new ArrayList<>();
    private List<Cells> deadCells = new ArrayList<>();
    private List<Cells> changedCells = new ArrayList<>();

    public void setAliveCells(List<Cells> aliveCells) {
        this.aliveCells = aliveCells;
        for (Cells cell : aliveCells) {
            cell.setState(Alive);
        }
    }

    public State getNextState(Cells cell, int liveNeighbours) {
        if ((cell.getCurrentState() == Alive) && (liveNeighbours < 2 || liveNeighbours > 3)) {
            cell.setNextState(Dead);
            changedCells.add(cell);
        } else if ((cell.getCurrentState() == Dead) && liveNeighbours == 3) {
            cell.setNextState(Alive);
            changedCells.add(cell);
        } else {
            cell.setNextState(cell.getCurrentState());
        }

        return cell.getNextState();
    }

    private void killCellsWithLessThanTwoOrMoreThanThreeNeighbours() {
        int aliveNeighbours;
        for (Cells currentCell : aliveCells) {
            aliveNeighbours = getNumberOfAliveCellsAroundLiveCells(currentCell);
            getNextState(currentCell, aliveNeighbours);
        }
    }


    public int getNumberOfAliveCellsAroundLiveCells(Cells centerCell) {
        setNeighboursForAliveCells(centerCell);
        return centerCell.aliveNeighbours.size();
    }

    private void setNeighboursForAliveCells(Cells centerCell) {
        List<Cells> neighboursList = centerCell.getNeighbours();
        deadCells.addAll(neighboursList);
        for (Cells cell : aliveCells) {
            for (Cells neighbour : neighboursList) {
                if (cell.equals(neighbour)) {
                    centerCell.aliveNeighbours.add(neighbour); // try this again
                    neighbour.setState(Alive);
                    deadCells.remove(centerCell);
                } else {
                    neighbour.setState(Dead);
                }
            }
        }
    }

    private void resurrectCellsWithThreeLiveNeighbours() {
        int aliveNeighbours;
        for (Cells potentialCell : deadCells) {
            aliveNeighbours = findNumberOfLiveCellsAroundDeadCells(potentialCell);
            getNextState(potentialCell, aliveNeighbours);
        }
    }

    public int findNumberOfLiveCellsAroundDeadCells(Cells potentialCell) {
        List<Cells> neighboursOfDeadCell = potentialCell.getNeighbours();
        for (Cells cell : aliveCells) {
            for (Cells neighbour : neighboursOfDeadCell) {
                if (cell.equals(neighbour)) {
                    potentialCell.aliveNeighbours.add(cell);
                }
            }
        }
        return potentialCell.aliveNeighbours.size();
    }

    public void applyAllRulesAndGenerateNextPattern() {
        killCellsWithLessThanTwoOrMoreThanThreeNeighbours();
        resurrectCellsWithThreeLiveNeighbours();
        setNextPattern();
    }

    private void setNextPattern() {
        for (Cells cell : changedCells) {
            cell.setState(cell.getNextState());
        }
    }

    public List<Cells> getAliveCellsInNextGeneration() {
        List<Cells> aliveCellsForNextGen = new ArrayList<>();
        List<Cells> consideredCells = new ArrayList<>();
        consideredCells.addAll(aliveCells);
        consideredCells.addAll(deadCells);
        for (Cells cell : consideredCells) {
            if (cell.getCurrentState() == Alive)
                aliveCellsForNextGen.add(cell);
        }
        aliveCellsForNextGen = removeDuplicates(aliveCellsForNextGen);
        return aliveCellsForNextGen;
    }

    private List<Cells> removeDuplicates(List<Cells> aliveCellsForNextGen) {
        Set<Cells> aliveCellsWithoutDuplicates = new HashSet<>(aliveCellsForNextGen);
        aliveCellsForNextGen.clear();
        aliveCellsForNextGen.addAll(aliveCellsWithoutDuplicates);

        return aliveCellsForNextGen;
    }

    @Override
    public String toString() {
        return "Game{" +
                "aliveCells=" + aliveCells +
                '}';
    }

    public static void main(String... a) {
        Scanner in = new Scanner(System.in);
        List<Cells> inputAliveCells = new ArrayList<>();
        char choiceOfInput;
        System.out.println("Enter the coordinates for Alive Cells:");
        do {
            System.out.println("Enter X Co-ordinate:");
            int xCoordinate = in.nextInt();
            in.nextLine();
            System.out.println("Enter Y Co-ordinate:");
            int yCoordinate = in.nextInt();
            in.nextLine();

            Cells cell = new Cells(xCoordinate, yCoordinate);
            inputAliveCells.add(cell);

            System.out.println("Do you want to add another Cell As Input: Y || N");
            choiceOfInput = in.nextLine().charAt(0);

        } while (choiceOfInput == 'Y' || choiceOfInput == 'y');

        Game game = new Game();
        System.out.println("Game Starts");

        game.setAliveCells(inputAliveCells);
        game.applyAllRulesAndGenerateNextPattern();

        List<Cells> aliveCellsInNextPattern = game.getAliveCellsInNextGeneration();

        for (Cells cell : aliveCellsInNextPattern) {
            System.out.println(cell.getX() + "," + cell.getY());

        }
    }
}

Cell.java:

package tw51.assignment.gameoflife;

import java.util.ArrayList;
import java.util.List;

/**
 * Represents A Cell with Position,Neighbours And State.
 */
public class Cells {


    private final int xCoordinate;
    private final int yCoordinate;
    private State state;
    private State nextState;
    public List<Cells> aliveNeighbours = new ArrayList<>();

    public int getX() {
        return xCoordinate;
    }

    public int getY() {
        return yCoordinate;
    }

    public enum State {Alive, Dead}

    public Cells(int xCoordinate, int yCoordinate) {
        this.xCoordinate = xCoordinate;
        this.yCoordinate = yCoordinate;
    }


    public List<Cells> getNeighbours() {

        List<Cells> neighbourList = new ArrayList<>();
        neighbourList.add(new Cells(xCoordinate - 1, yCoordinate - 1));
        neighbourList.add(new Cells(xCoordinate - 1, yCoordinate));
        neighbourList.add(new Cells(xCoordinate - 1, yCoordinate + 1));
        neighbourList.add(new Cells(xCoordinate, yCoordinate - 1));
        neighbourList.add(new Cells(xCoordinate, yCoordinate + 1));
        neighbourList.add(new Cells(xCoordinate + 1, yCoordinate - 1));
        neighbourList.add(new Cells(xCoordinate + 1, yCoordinate));
        neighbourList.add(new Cells(xCoordinate + 1, yCoordinate + 1));
        return neighbourList;

    }

    public void setNextState(State nextState) {
        this.nextState = nextState;
    }

    public State getNextState() {
        return this.nextState;
    }


    public void setState(State stateOfCell) {
        this.state = stateOfCell;
    }

    public State getCurrentState() {
        return this.state;
    }

    @Override
    public boolean equals(Object o) {
        if (this == o) return true;
        if (o == null || getClass() != o.getClass()) return false;

        Cells cells = (Cells) o;

        if (xCoordinate != cells.xCoordinate) return false;
        if (yCoordinate != cells.yCoordinate) return false;
        return true;
    }

    @Override
    public int hashCode() {
        int result = xCoordinate;
        result = 31 * result + yCoordinate;
        return result;
    }
}
\$\endgroup\$

1 Answer 1

2
\$\begingroup\$

Regarding the Cells class:

  1. It is an implementation detail, and should not be public.
  2. As such, you need not even add getters and setters for it.
  3. x and y is as clear as xCoordinate.
  4. Keeping the previous and next state inside the class is dubious in my opinion. I'd make the container keep previous and next state.
  5. Keeping x and y as part of the cell is an open door to duplicate cells, in the collection, which forced you to handle this case explicitly. It also makes you keep redundant information about neighbors. A much more simple and "obvious" way is to keep a mapping from coordinates to state in the Game class.
  6. The State fields are not initialized.

The Game class itself is both the board and the rules - it is not cohesive.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

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