Java console version Conway's Game of Life

I did a super simple version of Conway's Game of Life and would like a review.

```
public class Conway {
String [][] grid;
int [][] neighbors;
int [][] directions = {{0,1},{1,1},{1,0},{1,-1},{0,-1},{-1,-1},{-1,0},{-1,1}};
Conway (int gridSize) {
this.neighbors = new int [gridSize][gridSize];
this.grid = new String[gridSize][gridSize];
for (int row = 0; row .9) ? "x|" : " |";
System.out.print(this.grid[row][cell]);
}
System.out.println();
}
this.getNeihbors();
}
void getNeihbors() {
int i = 0;
for (int row = 0; row this.grid.length) {
x -= x;
}
if (y this.grid[row].length) {
y -= y;
}
if (this.grid[x][y] == "x|") i++;
}
this.neighbors[row][col] = i;
i=0;
}
}
this.showGrid();
}
void showGrid() {
for (int row = 0; row 3) {
this.grid[row][cell] = " |";
} else if (this.grid[row][cell] == "x|" && this.neighbors[row][cell] < 2) {
this.grid[row][cell] = " |";
}
System.out.print(this.grid[row][cell]);
}
System.out.println();
}
this.getNeihbors();
}
public static void main(String[] args) {
Conway c = new Conway(80);

Object fields

String [][] grid;
    int [][] neighbors;

I would make both of these private rather than the default package-protected visibility. Unless of course you are actually planning to access these from another class in the package.

private String [][] grid;
    private int [][] neighborCounts;

I also changed the name of neighbors, as I found the original inaccurate. I would expect a variable called neighbors to hold some kind of reference to the neighbors. Instead, this counts the neighbors, which is necessary for Conway's Game of Life. So let's call it what it is.

I'd also consider changing the grid to be some other kind of object rather than String. But that's something of a judgment call. One advantage of doing that would be that you could get rid of neighbors altogether. Instead, each cell would track its own neighbors.

Class constants

int [][] directions = {{0,1},{1,1},{1,0},{1,-1},{0,-1},{-1,-1},{-1,0},{-1,1}};

You make this a regular object field, but it never changes and is the same for all instances of the class.

private static final int [][] directions
                = {{0,1}, {1,1}, {1,0}, {1,-1}, {0,-1}, {-1,-1}, {-1,0}, {-1,1}};

Now only one copy will be initialized for the application.

Constructor initialization

this.neighbors = new int [gridSize][gridSize];
                this.grid = new String[gridSize][gridSize];

Personally, I think that eight-column indent is a bit much.

grid = new String[gridSize][gridSize];
            neighborCounts = new int[grid.length][grid[0].length];

Four-column indent is more common in Java.

If you do use an eight-column indent, consider adopting the same rule as the Linux source: only two levels of indent inside a function. That will help keep function contents from shooting off the edge. It also helps keep functions simple, without too much nested logic.

I prefer not to use the this. with object fields. Java doesn't require it, so I only use it when necessary to disambiguate from parameters with the same name.

You want neighborCounts to be the same size as grid. So make sure of that by setting it to the same dimensions dependently rather than in parallel. Parallel logic is fragile.

Don't run inside the constructor

At the end of the constructor, you

this.getNeihbors();

First, that's misspelled (missing a 'g'). It should be getNeighbors.

Second, it's not idiomatic. I would expect a method called getNeighbors to return the contents of an object field neighbors which holds the neighbors of something. This fills out the neighbors multidimensional array. I would probably call it countNeighbors.

Third, if you start the simulation from the constructor, then you have no way of constructing an object without starting the simulation.

Consider removing all current calls to getNeighbors and showGrid and instead adding to main something like

while (true) {
            c.countNeighbors();
            c.showGrid();
        }

That also has the advantage of not filling up the stack with recursive calls.

Simplifying

for (int drow = 0; drow < this.directions.length; drow++) {
                                        int x = (this.directions[drow][this.directions[drow].length-this.directions[drow].length]);
                                        int y = (this.directions[drow][this.directions[drow].length - 1]);
                                        x = row + x;
                                        y = col + y;

This could be as simple as

for (int[] direction : directions) {
                    int y = row + direction[0];
                    int x = col + direction[1];

And

} else if (x + 1 > this.grid.length) {
                                                 x -= x;

could be

} else if (x >= grid.length) {
                        x -= grid.length;

Now if you change the rules to allow neighbors that aren't adjacent, this will still work.

And we don't have to do an extra, unnecessary math operation.

Descriptive names

int i = 0;

I would find this easier to follow as

int count = 0;

When I see a variable named i, I expect it to be a loop iteration variable. This isn't. We're counting the neighbors. Why not name it based on that?

StackExchange Code Review Q#145150, answer score: 3