Search code examples
javaoptimizationcyclomatic-complexity

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


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).


Solution

  • 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)