patternjavaModerate
Checking for neighbours more elegantly in Conway's Game of Life
Viewed 0 times
checkingmoreconwayneighboursgameelegantlyforlife
Problem
My method for counting neighbors in my soon-to-be Game of Life implementation is very repetitive and I was wondering if this could be done more elegantly:
Because officially the board should be infinite, I don't need out-of-bounds checks in this code - I assume the board is infinite here -, but my Board implementation secretly looks like this:
(Just to save you the time it costs to type 'your code will cause an
I'm stuck with Java 6 by the way.
static int countNeighbours(Board b, int x, int y) {
int count = 0;
if (b.getTile(x - 1, y - 1)) {
++count;
}
if (b.getTile(x, y - 1)) {
++count;
}
if (b.getTile(x + 1, y - 1)) {
++count;
}
if (b.getTile(x + 1, y)) {
++count;
}
if (b.getTile(x + 1, y + 1)) {
++count;
}
if (b.getTile(x, y + 1)) {
++count;
}
if (b.getTile(x - 1, y + 1)) {
++count;
}
if (b.getTile(x - 1, y)) {
++count;
}
return count;
}Because officially the board should be infinite, I don't need out-of-bounds checks in this code - I assume the board is infinite here -, but my Board implementation secretly looks like this:
// TODO: make the board 'infinite'
public boolean getTile(int x, int y) {
if (x getWidth() - 1 || y > getHeight() -1) return false;
return tiles[y][x];
}
public void setTile(int x, int y, boolean val) {
if (x getWidth() - 1 || y > getHeight() -1) return;
tiles[y][x] = val;
}(Just to save you the time it costs to type 'your code will cause an
IndexOutOfBoundsException for the tile on (0,0)'. This question is on the implementation of static int countNeighbours(Board, int, int).)I'm stuck with Java 6 by the way.
Solution
You're right, there's an alternate way to do this, but, first, some Java standards:
-
1-liners should have braces. This is a code-style that is common to many langauges because it is more maintainable, and leads to fewer future bugs. Lines like:
should be:
-
Don't use arithmetic when you can use a simpler operator. Back to:
Which can be simplified to:
-
Use plain booleans when they make sense (again, back to...):
which can be put in a function like (transformed to uses && instead):
Note, how the standard boolean short-circuit evaluation ensures that the x and y are valid before the
OK, about that code duplication. A trick with these sorts of problems is to use an array of offsets. Consider your grid, which has 8 neighbours (in a relative
We can put these neighbour offsets in to an array:
Then, your check method becomes:
-
1-liners should have braces. This is a code-style that is common to many langauges because it is more maintainable, and leads to fewer future bugs. Lines like:
if (x getWidth() - 1 || y > getHeight() -1) return;should be:
if (x getWidth() - 1 || y > getHeight() -1) {
return;
}-
Don't use arithmetic when you can use a simpler operator. Back to:
if (x getWidth() - 1 || y > getHeight() -1) { ... }Which can be simplified to:
if (x = getWidth() || y >= getHeight()) { ...}-
Use plain booleans when they make sense (again, back to...):
if (x = getWidth() || y >= getHeight()) { ...}which can be put in a function like (transformed to uses && instead):
public boolean getTile(int x, int y) {
return x >= 0 && y >= 0 && x < getWidth() && y < getHeight() && tile[y][x];
}Note, how the standard boolean short-circuit evaluation ensures that the x and y are valid before the
tile[y][x] is used.OK, about that code duplication. A trick with these sorts of problems is to use an array of offsets. Consider your grid, which has 8 neighbours (in a relative
y,x format):-1,-1 -1,0 -1,+1
0,-1 *us* 0,+1
+1,-1 +1,0 +1,+1We can put these neighbour offsets in to an array:
private static final int[][] NEIGHBOURS = {
{-1, -1}, {-1, 0}, {-1, +1},
{ 0, -1}, { 0, +1},
{+1, -1}, {+1, 0}, {+1, +1}};Then, your check method becomes:
static int countNeighbours(Board b, int x, int y) {
int cnt = 0;
for (int[] offset : NEIGHBOURS) {
if (b.getTile(x + offset[1], y + offset[0]) {
cnt++;
}
}
return cnt;
}Code Snippets
if (x < 0 || y < 0 || x > getWidth() - 1 || y > getHeight() -1) return;if (x < 0 || y < 0 || x > getWidth() - 1 || y > getHeight() -1) {
return;
}if (x < 0 || y < 0 || x > getWidth() - 1 || y > getHeight() -1) { ... }if (x < 0 || y < 0 || x >= getWidth() || y >= getHeight()) { ...}if (x < 0 || y < 0 || x >= getWidth() || y >= getHeight()) { ...}Context
StackExchange Code Review Q#62160, answer score: 18
Revisions (0)
No revisions yet.