Conway's Game of Life logic error

Question

I'm taking a class that uses C# and our first assignment is to implement Conway's game of Life. We must do this by reading in a text file formatted something like this:

 *
  *
***

          ***

We then have to display the next 10 generations on the screen. I have the file read into a string array, then I copy it to another array. I then go through it character by character and change the copied array to match what the next generation should be. My problem is that the code I have to count the live neighbors isn't working and I can't figure out why. I displayed the number of live neighbors for each cell on the screen and about half of them are wrong. I know the error is occurring with cells on the edge of the "board," but I can't figure out how to fix it.

Now, I don't want the whole thing written for me, that'd be a bit pointless. I just can't figure out where my logic is off. Any help would be appreciated. Also, I'm aware that my code is pretty poor, overall. This was just the only way I could figure it out. Sorry.

 class Program
{
    static void Main(string[] args)
    {
        //gets file name from command arguments
        //checks to make sure file exists, exits if file does not exist
        if (!File.Exists(Environment.GetCommandLineArgs()[1])) { System.Environment.Exit(1); }

        //gets file name from command arguments then reads file into array of strings
        string[] gen0 = File.ReadAllLines(Environment.GetCommandLineArgs()[1]);

        string[] gen1 = gen0;
        char alive = '*';
        char dead = ' ';

        //displays first generation
        foreach (string s in gen0)
        {
            Console.WriteLine(s);
        }
        Console.WriteLine("=====================================");
        //counts live neighbors of a cell
        int count = 0;
        for (int i = 0; i < gen0.Length; i++)
        {
            count = 0;
            for (int j = 0; j < gen0[i].Length; j++)
            {
                //check top left neighbor
                if (i > 0 && j > 0 && j < gen0[i-1].Length )
                {
                    if (gen0[i - 1][j - 1] == alive) { count++; }
                }
                //check above neighbor
                if (i > 0 && j < gen0[i-1].Length)
                {
                    if (gen0[i - 1][j] == alive) { count++; }
                }
                //check top right neighbor
                if (i > 0 && j + 1 < gen0[i - 1].Length)
                {
                    if (gen0[i - 1][j + 1] == alive) { count++; }
                }
                //check left neighbor
                if (j > 0)
                {
                    if (gen0[i][j - 1] == alive) { count++; }
                }
                //check right neighbor
                if (j + 1 < gen0[i].Length)
                {
                    if (gen0[i][j + 1] == alive) { count++; }
                }
                //check bottom left neighbor
                if (i + 1 < gen0.Length && j > 0 && j < gen0[i+1].Length)
                {
                    if (gen0[i + 1][j - 1] == alive) { count++; }
                }
                //check below neighbor
                if (i + 1 < gen0.Length && j < gen0[i+1].Length)
                {
                    if (gen0[i + 1][j] == alive) { count++; }
                }
                //check bottom right neighbor
                if (i + 1 < gen0.Length && j + 1 < gen0[i].Length && j + 1 < gen0[i+1].Length)
                {
                    if (gen0[i + 1][j + 1] == alive) { count++; }
                }

                //Console.WriteLine(count); 
                //kills cells
                if (count < 2 || count > 3) 
                {
                    gen1[i] = gen1[i].Remove(j, 1);
                    gen1[i] = gen1[i].Insert(j, dead.ToString()); 
                }
                //births cells
                if (count == 3)
                {
                    gen1[i] = gen1[i].Remove(j, 1);
                    gen1[i] = gen1[i].Insert(j, alive.ToString());
                }
            }
        }
        foreach (string s in gen1)
        {
            Console.WriteLine(s);
        }
    } 
}

score 2 · Answer 1 · answered Sep 12 '13 at 15:46

Your problem is very simple - you're resetting count in the wrong place. Put it inside the loop and it will (probably) work.

Regarding the rest of the code, if you want to make it significantly easier to follow just give your game area a one-element border.

You'll need to pad the file you read in (blank line above and below, blank characters to the left and right), and alter your loops to:

  for (int i = 1; i < gen0.Length - 1; i++)

and

  for (int j = 1; j < gen0[i].Length - 1; j++)

but your central count calculation can then be reduced down to a single calculation:

  count = (gen0[i - 1][j - 1] == alive) ? 1 : 0 +
          (gen0[i - 1][j] == alive) ? 1 : 0 +

          ... etc ...

which should make for cleaner code and ensure that any other errors you may make are significantly easier to spot.

Yes, and if the string arrays were replaced by 2-D int arrays the logic would become still easier: `count = board[i - 1, j - 1] + board[i - 1, j] + ...`, where `1` would mean *alive* and `0` *dead* — Olivier Jacot-Descombes, Sep 12 '13 at 15:54
@OlivierJacot-Descombes Much as comments aren't meant to be "thanks" or "+1"s, you're right - that would be a neat way to do it with just a little bit of processing of the text file before the main loop starts. — Adrian Wragg, Sep 12 '13 at 16:14

score 2 · Answer 2 · answered Sep 12 '13 at 15:55

2

So the first bug I see is that you don't actually copy the board for the next iteration.

gen1 = gen0;

The code above only assigns the gen1 reference to the same object as gen0. So when you modify gen1, you actually modify gen0 as well... causing inconsistencies in latter iterations. Try this:

gen1 = (string[])gen0.Clone();

Second bug is that int count = 0 should be in the second loop instead of first:

for (int i = 0; i< gen0.Length; i++)
{
    // not here
    // int count = 0
    for (int j = 0; j < gen0[i].Length; j++)
    {
        // here
        int count = 0
        ...
    }
...
}

This way you reset the count for every cell instead of every row.

answered Sep 12 '13 at 15:55

Vlad

1,889
17
33

Is there a way to make each row in gen1 the length of the longest row in gen0 before copying the it? – user2773218 Sep 12 '13 at 19:53
@user2773218 Sure, `int longestRow = gen0.Max(item => item.Length);` add blanks to every string until you reach that value. After, you can begin cloning/modifying gen0. Complication here is that you use an array of strings. I would just make a 2 dimensional character array instead, with pre-defined bounds and one cell padding on all sides, where every row size is the same. It will also save you from the headaches of doing the neighbour checks on uneven rows and edges. – Vlad Sep 12 '13 at 20:03

Conway's Game of Life logic error

2 Answers2