Game of Life: find neighbours

Question

I'm making a Life game and I made this method to find nearby neighbours

private int getNeighbours(LifeBoard board, int row, int col){
    if(board.get(row+1, col)){
        neighbours++;
    }
    if(board.get(row-1, col)){
        neighbours++;
    }
    if(board.get(row, col+1)){
        neighbours++;
    }
    if(board.get(row, col-1)){
        neighbours++;
    }
    if(board.get(row+1, col+1)){
        neighbours++;
    }
    if(board.get(row-1, col-1)){
        neighbours++;
    }
    if(board.get(row+1, col-1)){
        neighbours++;
    }
    if(board.get(row-1, col+1)){
        neighbours++;
    }

    return neighbours;
}

I feel as if it's horribly coded and cringe at it, therefore my question is.. Is there a way to make this better? Right now, it "sort-of" works but I'm thinking if I could do this with a loop instead.

Thanks.

@Jon Skeet has the right answer. However if you start having a big field your original way will execute faster because you basically unrolled the loop and loop unrolling is always faster than a loop. — markbernard, Nov 24 '14 at 16:44

Jon Skeet · Accepted Answer · 2014-11-24T16:22:27.203

Well, you can use loops and just explicitly exclude the location itself (i.e. when the x and y offset are both 0):

private int getNeighbours(LifeBoard board, int row, int col) {
    int neighbours = 0;
    for (int xOffset = -1; xOffset < 2; xOffset++) {
        for (int yOffset = -1; yOffset < 2; yOffset++) {
            if ((xOffset != 0 || yOffset != 0)
                  && board.get(row + yOffset, col + xOffset)) {
                neighbours++;
            }
        }
    }
    return neighbours;
}

This assumes your board.get(...) method is okay with values off the edge of the board.

Alternative strategies for the offsets:

As above, for (int xOffset = -1; xOffset < 2; xOffset++)
Use an inclusive upper bound: for (int xOffset = -1; xOffset <= 1; xOffset++)

Use an array defined elsewhere:

private static final int[] OFFSETS = { -1, 0, 1 };
...
for (int xOffset : OFFSETS)

I would use something like `OFFSETS = new int[] {-1, 0, 1}` and `for (int xOffset : OFFSETS) {...}` — njzk2, Nov 24 '14 at 16:19

Game of Life: find neighbours

1 Answers1