0
\$\begingroup\$

I ended up using 2x 2D arrays with a one element border around everything to simplify the kernel logic.

import android.graphics.Canvas;
import android.graphics.Color;
import android.graphics.Paint;

import java.util.Random;

public class WorldSimulation {

    long generation = 0;

    int[][]  next = null;
    int[][] current = null;

    float cellWidth;
    float cellHeight;

    int width;
    int height;

    public WorldSimulation(int width, int height, float w, float h)
    {
        this.width = width;
        this.height = height;

        next = new int[width+2][height+2];
        current = new int[width+2][height+2];

        cellWidth = (w / this.width);
        cellHeight = (h / this.height);
    }

    public void timeStep(Canvas canvas, Paint paint)
    {
        generation ++;

        for (int i = 0; i < next.length; i++) {
            System.arraycopy(next[i], 0, current[i], 0, next[0].length);
        }

        for(int i = 1; i < this.width+1; i++)
        {
            for(int j = 1; j < this.height+1; j++)
            {
                float left = (i * cellWidth);
                float top = (j * cellHeight);
                float right = (left + cellWidth);
                float bottom = (top + cellHeight);

                int status = current[i][j];

                int neighbours = livingNeighbours(i, j);

                if(status == 255) {
                    paint.setColor(Color.rgb(0, 255, 0));

                    canvas.drawRect(left-cellWidth,top-cellHeight,right,bottom,paint);

                    if (neighbours < 2) {
                        next[i][j]=0;
                    }
                    else if(neighbours >3)
                    {
                        next[i][j]=0;
                    }
                }
                else if(status!=255)
                {
                    paint.setColor(Color.rgb(0, status--, 0));
                    canvas.drawRect(left-cellWidth,top-cellHeight,right,bottom,paint);

                    if(neighbours == 3)
                    {
                        next[i][j] =  255;
                    }
                }
            }
        }
    }

    public int livingNeighbours(int i, int j) {
        int alive = 0 ;

        for(int kernelRow = i -1; kernelRow <= i+1; kernelRow ++)
        {
            for(int kernelCol = j -1; kernelCol <= j+1; kernelCol ++)
            {
                if(current[kernelRow][kernelCol]== 255 && !(kernelRow == i && kernelCol == j))
                {
                    alive ++;
                }
            }
        }
        return alive;
    }

    public void setCell(int i, int j, boolean alive)
    {
        next[i][j] =  255;
    }

}

The full implementation can be found here: https://github.com/jacasey/life/ and I've put a version up on the Google Play Store as well.

The whole thing clocks in at around 12 KB which I think is pretty cool when you compare it to the monters apps on the playstore today.

I was thinking of re-implementing this with a 1D array and indexing calculations but I don't think this would really add to much re: size or performance wise.

https://play.google.com/store/apps/details?id=com.jcasey.life

\$\endgroup\$
1
\$\begingroup\$

General isues

Your code doesn't follow the single responsibility principle. The class WorldSimulation is responsible for updating the game and creating the graphical representation. If you followed a naming convention where the name of a class describes it's functionality it would have to be WorldSimulationAndPaint.

Instead of passing the canvas and paint to the game engine, you should try to follow the MVC pattern. I have once implemented a Conway's game of life too. I had a timer that was responsible for controlling the updates. The timer sent an event to the game engine telling it "please advance the game state by one frame". When the game engine had done that, the game engine sent an event to the user interface telling it "here's the lates game state, please update the user interface."

Once you have removed the responsibility of drawing from WorldSimulation you can remove the cellWidth and cellHeight fields.

The events can be transmitted by setting up each component as a listener to the relevant source or with an event bus (such as Google Guava).

Field visibility

All the fields in WorldSimulation are visible to other classes in the package. The fields of a class should be as closely guarded as possible so they should be private unless a there is a very good reason to do otherwise. That way you guarantee that the state of the object can only be changed by the object itself. It also forces you to concentrate on the interface your class provides to others.

The livingNeighbours method is not supposed to be accessed from outside the class, so it should be private too.

Magic numbers

What does 255 mean? Instead of using a literal integer every time you should define it as a constant whose name describes the meaning of the value and use the constant instead of the literal 255.

private static final int STATUS_ALIVE = 255;
private static final int STATUS_DEAD = 0;

Duplicated code

The drawRect call is identical in both branches of the if statement. You can take them both out and replace them with a single call after the if statement.

canvas.drawRect(left-cellWidth,top-cellHeight,right,bottom,paint);

Misleading naming

In livingNeighbours you count number of cells, but the value is collected in a variable named alive. In spoken language "alive" has only two values (alive or dead). Thus it sugests that the field would be a boolean (as you have done in the setCell method), while it is in fact an integer. It should be named livingNeighbourCount.

Misleading method signature

The alive parameter suggest that one could set a cell to be either living or dead. The implementation however only sets it to a living state. Either fix the code to handle the parameter or remove it and rename the method to setCellAlive.

public void setCell(int i, int j, boolean alive)
{
    next[i][j] =  255;
}

Rules

You're not really implementing Conway's Game of Life. :)

|improve this answer|||||
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.