Reduce complexity of counting neighbours (Conway's Game of Life)

Question

I have to implement Conway's Game of Life. Everything works as it should and given tests are passing. My only problem is that this method gives complexity error while running PMD rules on my file. I understand that so many if sentences are the cause of that, but while trying to compact them into smaller groups I accidentally broke my code.

Here's what it says:

The method 'getNeighbourCount(int, int)' has a cyclomatic complexity of 21.

The method 'getNeighbourCount(int, int)' has an NPath complexity of 20736, current threshold is 200

What would best the best options for optimizing this method?

public Integer getNeighbourCount(int x, int y) {
        // x = column (20), y = row (15)

        int countNeigbours = 0;
        if (x != 0 && y != 0 && isAlive(x - 1,y - 1)) {
            countNeigbours++;
        }

        if (x != 0 && isAlive(x - 1, y)) {
            countNeigbours++;
        }

        if (x != 0 && y != rows - 1 && isAlive(x - 1,y + 1)) {
            countNeigbours++;
        }
        if (y != 0 && isAlive(x,y - 1)) {
            countNeigbours++;
        }
        // check self
        if (y != rows - 1 && isAlive(x,y + 1)) {
            countNeigbours++;
        }

        if (x != columns - 1 && y != 0 && isAlive(x + 1,y - 1)) {
            countNeigbours++;
        }

        if (x != columns - 1 && isAlive(x + 1, y)) {
            countNeigbours++;
        }

        if (x != columns - 1 && y != rows - 1 && isAlive(x + 1,y + 1)) {
            countNeigbours++;
        }

        return countNeigbours;
}

isAlive returns the boolean if the cell is taken (true) or not (false).

score 0 · Accepted Answer · answered Apr 06 '21 at 15:31

Loop over the "delta x" and "delta y" from your current position:

for (int dx : new int[]{-1, 0, 1}) {
  if (x + dx < 0 || x + dx >= columns) continue;

  for (int dy : new int[]{-1, 0, 1}) {
    if (y + dy < 0 || y + dy >= rows) continue;
    if (dx == 0 && dy == 0) continue;

    if (isAlive(x + dx, y + dy)) countNeighbours++;
  }
}

(Of course, you don't have to use arrays and enhanced for loops: you can just do for (int dx = -1; dx <= 1; ++dx), or however else you like)

Thank you, it really helped me. First, it was tricky to understand, but as debugged my code it made sense :) — Meowz, Apr 13 '21 at 19:13

Thomas Bitonti · Answer 2 · 2021-04-06T18:20:55.393

I don't know if this would provide a speed-up, but you could try having a second array which held the sums, and increased or decreased these values when setting or clearing individual cells. That replaces the many 'isAlive' checks with a check to tell if a cell should be toggled on or off, and reduces the cell adjacency computations to just those cells which were toggled, which should be many fewer than repeating the computation for the entire array. That is, for a mostly empty grid, only a small subset of cells should require recomputation, many fewer than the entire grid.

Also, you could try having whole row isActive, minActive, and maxActive values. That would further reduce the amount of looping, but would increase the complexity and cost of each iteration. The sparseness of active cells would determine whether the extra cost was balanced by the reduction in the number of iterations.

Reduce complexity of counting neighbours (Conway's Game of Life)

2 Answers2