HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaMinor

Game of Life Neighbour count optimization

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
optimizationgameneighbourlifecount

Problem

This is a working Neighbours method. Is there any way to make this simpler and concise?

public int Neighbours(int xCoordinate, int yCoordinate)
        {
            xCoordinate = xCoordinate -1;
            yCoordinate = yCoordinate -1;
            int NeighbourCounter =0;
            int incrementer = 0;
            int i1 =-1; 
            int j1 =-1;
            int i2 =1; 
            int j2 =1; 

            if(yCoordinate==0)
            {
                j1=1;
            }

            if(xCoordinate==0)
            {
                i1=1;
            }

            if(yCoordinate+1==xLen)
            {
                j2=0;
            }

            if(xCoordinate+1==yLen)
            {
                i2=0;
            }

            for(int j=j1; j<=j2; j++)
            {
                if(j==0)
                {
                    incrementer = 2;
                }
                else
                {
                    incrementer = 1;
                }

                for(int i=i1; i<=i2; i+=incrementer)
                {
                    if(CG[yCoordinate + j][xCoordinate + i] == 1)
                    {
                        NeighbourCounter++;
                    }
                }
            }
                return NeighbourCounter;
        }

Solution

There are several style issues with this code. Most notably:

  • Use camelCase for method names



  • Use camelCase for variable names



  • Put spaces around operators, for example:



  • i = i1 instead of i=i1



  • yCoordinate + 1 == xLen instead of yCoordinate+1==xLen



  • ... and so on



  • j1, j2, ... are poor variable names



To simplify this code, a couple of ideas:

  • Use an enum to represent neighbors: North, NorthEast, East, SouthEast, ...



  • Create custom fields dx and dy to represent the direction offset



  • Iterate over Neighbor.values(), for each item perform this check:



  • Calculate the neighbor position using current position + the neighbor's dx and dy



  • Check if the position is on the grid, for example with a isValid(Position pos, Neighbor neighbor) method



  • Check if the position is alive, for example with a isAlive(Position pos, Neighbor neighbor) method



  • If both the above are true, increment the count



To get you started, here's an example enum:

enum Neighbor {
    North(0, 1),
    NorthEast(1, 1),
    East(1, 0);

    private final int dx;
    private final int dy;

    Neighbor(int dx, int dy) {
        this.dx = dx;
        this.dy = dy;
    }
}

Code Snippets

enum Neighbor {
    North(0, 1),
    NorthEast(1, 1),
    East(1, 0);

    private final int dx;
    private final int dy;

    Neighbor(int dx, int dy) {
        this.dx = dx;
        this.dy = dy;
    }
}

Context

StackExchange Code Review Q#70864, answer score: 6

Revisions (0)

No revisions yet.