A function for counting neighbors in a multidimensional array for the Game of Life game

Question

i writing Game of Life using c++ and multidimensional array for this. its code:

#include <iostream>
#include <windows.h>
#include <cstdlib>

using namespace std;


const int mapSize = 10;
int neighborhoods = 0;
int isLife = 0;
int numSys = 0;

void ShowMap(int map[mapSize][mapSize], int size);
void Check(int map[mapSize][mapSize], int size);
int neighborhood(int map[mapSize][mapSize], int x, int y);


int main() {

    int map[mapSize][mapSize] = {{0,0,0,0,0,0,0,0,0,0},
                                 {0,0,0,0,0,0,0,0,0,0},
                                 {0,0,0,0,0,0,0,0,0,0},
                                 {0,0,0,0,0,1,0,0,0,0},
                                 {0,0,0,0,0,1,0,0,0,0},
                                 {0,0,0,0,0,1,0,0,0,0},
                                 {0,0,0,0,0,0,0,0,0,0},
                                 {0,0,0,0,0,0,0,0,0,0},
                                 {0,0,0,0,0,0,0,0,0,0},
                                 {0,0,0,0,0,0,0,0,0,0},}; 
    for(int i = 0;i > mapSize; i++) {
        map[i][i+2] = 1;
    } 
    int userRow, userColumn;

    while(1) { 
        ShowMap(map, mapSize);
        Check(map, mapSize);
        Sleep(1000);

    }
    };
        




void ShowMap(int map[mapSize][mapSize],  int size) {
    cout<< endl;
    for(int i = 0;i < size; i++) {
        for(int j = 0; j < size; j++) {

            cout<< map[i][j] << " ";
        };
        cout<< endl;
    }
    cout<< endl;
}


void Check(int map[mapSize][mapSize], int size) {
    isLife = 0;
    for(int i = 0;i < size; i++) {
        for(int j = 0; j <  size; j++) {
            if(map[i][j] == 1) {
                neighborhood(map, i, j);
                ++isLife;
            } else {

                cout << "nope" << "    ";
                neighborhood(map, i, j);
            }
        };
    }
    if(isLife <= 0) {
        
    } else {
        cout<< "Life: " << ifLife;
    }
    
}

int neighborhood(int map[mapSize][mapSize], int x, int y) {
    numSys = 0;
    for (int i = -1; i <= 1; i++) {
        for (int j = -1; j <= 1; j++) {
            int newX = x + i;
            int newY = y + j;
            if(newX == x && newY == y) {
                continue;
}
            if (newX >= 0 && newX < mapSize && newY >= 0 && newY < mapSize) {
                if (i == 0 && j == 0) {
                    continue;
                }
                if(map[newX][newY] == 1) {
                    ++numSys;
                } else {
                    continue;
                }


            }
        }
    }


    if(map[x][y] == 1 && (numSys == 2 || numSys == 3)) {
        cout<<endl<< "stay" << "   " << numSys;

    } 
    if(map[x][y] == 1 && numSys < 2) {
        cout<<endl<< "deppression" << "   " << numSys;
        map[x][y] = 0;
    } 
    if(map[x][y] == 1 && numSys > 3) {
        cout<<endl<< "overpopulation"<< "   " << numSys;
        map[x][y] = 0;
    } 
    if(map[x][y] == 0 && (numSys == 2 || numSys == 3)) {
        cout<<endl<<"New life"<< "   "<< numSys;
        map[x][y] = 1;
    }

But neighborhood function does not work as I need, it does not correctly count neighboring living cells, how can I fix this?

P.S:Sorry for my english

I thought that this function will count the neighboring 8 cells, but it works in a completely different way

Answer 1

There are a few issues in your code:

• The main issue is that by updating the matrix for a row i, you destroy the previous data you will need to properly calculate the neighborhood for the next row at i+1. The neighborhood check in row i+1 needs to count the alive cells on row i before you had updated them.

  A simple solution is to use two matrices and after you have written the new cell values in the second matrix, you can use that one for display and further processing, or you can copy that second matrix back into the first one.

  A bit more cautious is to use only 2 rows as extra memory, and write a newly calculated row back to your matrix as soon as you no longer need the original values in that row.

• The last if block in neighborhood can also kick in when one of the previous if blocks had been executed so that the cell is first set to 0 and then back to 1 again. This is undesired and leads to wrong results. These cases should be mutually exclusive: at most one of those if blocks should execute.

• neighborhood has logic to make a dead cell alive when it has 2 live neighboring cells, but this is not the rule in Conway's Game of Life. If a cell has two live neighbors, its state should not change: a dead cell should remain dead in that case.

• the for loop in main will not make any iterations. The end-condition is wrong (> should be <). If you correct that condition, then the loop will run into an error because i+2 will get out of range. So the condition needs to be further tuned. However, I don't really get why you would want to "draw" a diagonal through your grid...

Then there are some other things that should be improved:

• Unused variables:

  • The global variable neighborhoods is never used.
  • The global variable isLife is updated, but never used for anything else than printing a debugging message.
  • The variables userRow and userColumn are never used.

  Drop all these variables.

• As isLife served no purpose, simplify Check by removing the if..else statement, and just call neighborhood at one place only.

• numSys should not be global variable. Define it as a local variable in neighborhood

• The parameter size is always given mapSize as value, so you don't actually need that parameter: remove those, and replace every size with mapSize

• In neighborhood the following if condition will never be true: (i == 0 && j == 0): this condition was already tested earlier on with if(newX == x && newY == y). So the second one can be removed from your code.

• The logic in neighborhood can be much simplified. The new value for the current cell is determined by the expression: numSys == 2 ? map[x][y] : numSys == 3.

• I would not have neighborhood update anything in the grid (for the reasons given in the very first bullet point), but have it return the cell's new value. Then it is up to the caller to store that value somewhere (in a temporary array) and write it to the grid when it is save to do so.

• I would use a more appropriate name for Check: it doesn't just check something; it really updates the grid. Maybe name it nextGeneration.

• Be aware that Sleep is Windows specific. If you have no intention to share the code and only run it on your Windows machine, this is not a problem. See Cross platform Sleep function for C++, but I'll not touch on that in this answer.

• In C++ it is preferable to use std::array or std::vector instead of plain arrays. But I'll not touch on that in this answer.

Updated code

int main() {
    // Initialise a map with a "glider" pattern:
    int map[mapSize][mapSize] = {{0,0,0,0,0,0,0,0,0,0},
                                 {0,0,0,0,0,0,0,0,0,0},
                                 {0,0,0,0,0,0,0,0,0,0},
                                 {0,0,0,0,0,0,0,0,0,0},
                                 {0,0,0,0,0,0,0,0,0,0},
                                 {0,0,0,0,0,0,0,1,0,0},
                                 {0,0,0,0,0,0,1,1,0,0},
                                 {0,0,0,0,0,0,1,0,1,0},
                                 {0,0,0,0,0,0,0,0,0,0},
                                 {0,0,0,0,0,0,0,0,0,0},}; 
    while (true) { 
        showMap(map);
        nextGeneration(map);
        Sleep(1000);
    }
};

void showMap(const int map[mapSize][mapSize]) {
    cout<< endl;
    for (int i = 0; i < mapSize; i++) {
        for (int j = 0; j < mapSize; j++) {
            cout << map[i][j] << " ";
        }
        cout << endl;
    }
    cout << endl;
}

void updateRow(int map[mapSize][mapSize], int newRows[2][mapSize], int i) {
    if (i--) { // Update previous row if it exists:
        for (int j = 0; j < mapSize; j++) {
            map[i][j] = newRows[i % 2][j];
        }
    }
}

void nextGeneration(int map[mapSize][mapSize]) {
    int newRows[2][mapSize]; // Buffer to store new cell values
    for (int i = 0; i < mapSize; i++) {
        for (int j = 0; j < mapSize; j++) {
            newRows[i % 2][j] = neighborhood(map, i, j);
        }
        // Commit the values of the previous(!) row in the matrix
        updateRow(map, newRows, i);
    }
    updateRow(map, newRows, mapSize);
}

int neighborhood(const int map[mapSize][mapSize], int x, int y) {
    // Use a local variable, and already subtract current cell so
    //    to simplify the loop logic:
    int numSys = -map[x][y]; 
    for (int i = -1; i <= 1; i++) {
        int newX = x + i; // earlier calculation and range-check:
        if (newX >= 0 && newX < mapSize) {
            for (int j = -1; j <= 1; j++) {
                int newY = y + j;
                if (newY >= 0 && newY < mapSize) {
                    numSys += map[newX][newY];
                }
            }
        }
    }
    // Simplified calculation of cell's new value:
    return numSys == 2 ? map[x][y] : numSys == 3;
}