GameOfLife on GDX

Question

I would like to know if I did everything right and if there a way to do it more simple and\or better?

GameOfLife - main

package com.mygdx.game;

import com.badlogic.gdx.ApplicationAdapter;
import com.badlogic.gdx.Gdx;
import com.badlogic.gdx.graphics.Color;
import com.badlogic.gdx.graphics.GL20;
import com.badlogic.gdx.graphics.OrthographicCamera;
import com.badlogic.gdx.graphics.glutils.ShapeRenderer;

import java.net.URISyntaxException;
import java.nio.file.Paths;
import java.util.ArrayList;
import java.util.List;

// http://www.conwaylife.com/wiki/Rulestring#Rules
public class GameOfLife extends ApplicationAdapter {
    ShapeRenderer shapeRenderer;
    Board board;
    int boardSize = 25;//rectangular
    final int CELL_SIZE = 10;//rectangular
    final int CELL_SCALE = CELL_SIZE;
    List<Integer> bornRule = new ArrayList<Integer>();
    List<Integer> survivalRule = new ArrayList<Integer>();
    OrthographicCamera camera;
    String preSet[] = {"Glider", "R-pentomino", "Diehard"};
    String rule = "3/12345";
    private int delay = 100;

public GameOfLife() throws URISyntaxException {
}


@Override
public void create () {
    camera = new OrthographicCamera();
    camera.setToOrtho(true);
    shapeRenderer = new ShapeRenderer();
    shapeRenderer.setProjectionMatrix(camera.combined);


    parseRules();


    try {
        board = new Board(boardSize, bornRule, survivalRule, Paths.get(ClassLoader.getSystemResource(preSet[2]).toURI()));
    } catch (Exception e) {
        e.printStackTrace();
    }
}

private void parseRules() {
    String[] split = rule.split("/");
    String[] born = split[0].split("");
    String[] survive = split[1].split("");
    initList(born, bornRule);
    initList(survive, survivalRule);
}

private void initList(String[] splitedString, List<Integer> list) {
    for (String s : splitedString) {
        list.add(Integer.parseInt(s));
    }
}

@Override
public void render () {

    Gdx.gl.glClearColor(1, 0, 0, 1);
    Gdx.gl.glClear(GL20.GL_COLOR_BUFFER_BIT);


    shapeRenderer.begin(ShapeRenderer.ShapeType.Filled);
    for (Coordinate coordinate : board.getCells().keySet()) {
        if (board.getCell(coordinate).getState() == State.ALIVE){
            shapeRenderer.setColor(Color.BLACK);
        }
        shapeRenderer.rect(
                10+(board.getCell(coordinate).getCoordinate().getX()*CELL_SCALE)+board.getCell(coordinate).getCoordinate().getX(),
                10+(board.getCell(coordinate).getCoordinate().getY()*CELL_SCALE)+board.getCell(coordinate).getCoordinate().getY(),
                CELL_SIZE, CELL_SIZE);

        shapeRenderer.setColor(Color.WHITE);
    }

    shapeRenderer.end();
    try {
        Thread.sleep(delay);
    } catch (InterruptedException e) {
        e.printStackTrace();
    }
    board.updateState();
}

@Override
public void dispose () {
    shapeRenderer.dispose();
}
}

Board

package com.mygdx.game;

import java.io.IOException;
import java.nio.charset.StandardCharsets;
import java.nio.file.Files;
import java.nio.file.Path;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.List;
import java.util.Map;

public class Board {
    private Map<Coordinate, Cell> cells;
    private List<Integer> bornRule;
    private List<Integer> surviveRule;
    private List<Coordinate> field = new ArrayList<>();
    private int boardSize;


    public Board(int bs, List<Integer> br, List<Integer> sr, Path p) throws IOException {
        readFile(p);
        boardSize = bs;
        bornRule = br;
        surviveRule = sr;
        cells = new HashMap<>();
        for (int i = 0; i < boardSize; i++) {
            for (int j = 0; j < boardSize; j++) {
                Coordinate coordinate = new Coordinate(i, j);
                State state = State.DEAD;
                if (field.contains(coordinate)){
                    state = State.ALIVE;
                }
                Cell cell = new Cell(coordinate, state);
                cells.put(coordinate, cell);
            }
        }
    }
    private  void readFile(Path filePath) throws IOException {
        List<String> allDocLines = new ArrayList<>();
        System.err.println(filePath);
        try {
            allDocLines = Files.readAllLines(filePath, StandardCharsets.UTF_8);
        } catch (IOException e) {
            e.printStackTrace();
        }
        int j = 0;
        for (String line : allDocLines) {
            if (line.isEmpty()) {
                continue;
            }
                String[] split = line.split(",");
                int v = Integer.parseInt(split[0]);
                int k = Integer.parseInt(split[1]);
                Coordinate c = new Coordinate(v, k);

                field.add(c);
        }
    }

    public Map<Coordinate, Cell> getCells() {
        return cells;
    }

    public Cell getCell(Coordinate c) {
        return cells.get(c);
    }

    public void setCells(Map<Coordinate, Cell> cells) {
        this.cells = cells;
    }

    public Board() {
    }

    public void updateState() {
        for (Coordinate coordinate : cells.keySet()) {
            Cell cell = cells.get(coordinate);
            int neighborsCount = countNeighbors(cell);
            boolean shouldBeChanged = checkRules(cell.getState(), neighborsCount);
            cell.setChangeState(shouldBeChanged);
        }
        for (Coordinate coordinate : cells.keySet()) {
            Cell cell = cells.get(coordinate);
            if(cell.isChangeState())
                cell.changeState();
            cell.setChangeState(false);

        }
    }

    private boolean checkRules(State s, int neighborsCount) {
        if(s == State.ALIVE){
            if (surviveRule.contains(neighborsCount))
                return false;
            else
                return true;
        }else{
            if (bornRule.contains(neighborsCount))
                return true;
            else
                return false;
        }
    }

    public int countNeighbors(Cell c){
        int x = c.getCoordinate().getX();
        int y = c.getCoordinate().getY();
        int count = 0;
        for (Direction direction : Direction.values()) {

//            if (c.getState(x + direction.dx, y + direction.dy)==State.ALIVE) {
//                ++count;
//            }
            int newX = Math.floorMod(x + direction.dx, boardSize);
            int newY = Math.floorMod(y + direction.dy, boardSize);
            Coordinate xy = new Coordinate(newX, newY);
            if (cells.get(xy).getState()==State.ALIVE) {
                ++count;
            }
        }
        return count;
    }
}

Cell

package com.mygdx.game;

public class Cell {
    private Coordinate coordinate;

    private State state;
    private boolean changeState;

    public boolean isChangeState() {
        return changeState;
    }

    public void setChangeState(boolean changeState) {
        this.changeState = changeState;
    }

    public Cell(Coordinate coordinate, State state) {

        this.coordinate = coordinate;
        this.state = state;
    }

    public Coordinate getCoordinate() {
        return coordinate;
    }

    public void setCoordinate(Coordinate coordinate) {
        this.coordinate = coordinate;
    }

    public State getState() {
        return state;
    }

    public void setState(State state) {
        this.state = state;
    }
    public void changeState() {
        if(state==State.ALIVE){
            state = State.DEAD;
        }else{
            state = State.ALIVE;
        }
    }
}

Coordinate

package com.mygdx.game;

public class Coordinate {
    private int x;
    private int y;

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

        Coordinate that = (Coordinate) o;

        if (x != that.x) return false;
        return y == that.y;
    }

    @Override
    public int hashCode() {
        int result = x;
        result = 31 * result + y;
        return result;
    }

    @Override
    public String toString() {

        return "Coordinate{" +
                "x=" + x +
                ", y=" + y +
                '}';
    }

    public int getX() {
        return x;
    }

    public void setX(int x) {
        this.x = x;
    }

    public int getY() {
        return y;
    }

    public void setY(int y) {
        this.y = y;
    }

    public Coordinate(int x, int y) {

        this.x = x;
        this.y = y;
    }
    public Coordinate() {}
}

State

package com.mygdx.game;

public enum State {
    ALIVE,
    DEAD
}

Direction

package com.mygdx.game;

enum Direction {
    NORTHWEST(-1, -1),
    NORTH(0, -1),
    NORTHEAST(1, -1),
    EAST(1, 0),
    SOUTHEAST(1, 1),
    SOUTH(0, 1),
    SOUTHWEST(-1, 1),
    WEST(-1, 0),
    ;

    final int dx;
    final int dy;

    Direction(int dx, int dy) {
        this.dx = dx;
        this.dy = dy;
    }
}

Answer 1

Thanks for sharing the code.

here are a few things I found:

Naming

Finding good names is the hardest part in programming, so always take your time to think about the names of your identifiers.

On the bright side you follow the Java Naming Conventions.

Do not abbreviate

At some places you use "short names" for variables like v and k in readFile(). There is no penalty for using long identifier names.

don't surprise your readers

This special case of v and k is even worse because you use unusual abbreviations for field coordinates.

Separation of Concerns

Some of the functionality is not in the place I would expect it.

reading input file

IMHO this should be in a separate class. This would enable you to change the source of the initial setup to a database or a webservice without any change in the Borad class.

Switching the actual state

You have the Cell class deciding what state is next. This means, the Cell class nut know the available states and how they follow each other.

This looks obvious for the basic "Game of Life" but there are variants like "Forrest fire" where we have 3 states...

calculating neighbor coordinate

You calculate the neighbor coordinates in the Board class. I'd do this in the Direction class. This way you would also avoid the public access to the fields in the Direction (which violates information hiding principle).

General coding

Don't do work in constructors

Especially do not call methods in other classes. Call methods in own class only if they are private or final. Also do not invoke the new operator in a constructor.

The reason is that doing the above makes your code inflexible. It violates the open - closed principle.

use "real" constants

A Constant in Java is not only final, it also must be static.

Answer 2

To add to @Timoty:

Update State Logic

This code is really 'messy'. Cells have a 'shouldBeChanged' etc.

I would simplyfiy is and just create a new board of cells based on the old one. After you filled it, set the new board state as the actual. This technique is known in graphics as 'double buffering' and makes it a lot easier.

Missing 'Rule' object

While you have a lot of sensible abstractions, I think you missed the 'Rule' abstraction. You pass around Strings and pass them as integer lists, never introduce the 'Rule'.

Optimizing neighbors calculation

You know on construction what cells are neighbors of each other. I would encode this information in the cell, trading some memory for execution performance and better readability.

Coordinate

Do you really need the Coordinate class? If a Cells knows it's neighbors, you can simply iterate over all cells for the board and over all neighbors for each cell.