my implementation of conway's game of life in C++

Question

I found Conway's game of life. I had heard of it, of course, but didn't think it would be so easy and fun to implement. Anyway, here's my code which I know needs a lot of polish:

#include <iostream>
#include <vector>
#include <windows.h>
#define SIZE 10
using std::vector;
struct Pos {
 int x;
 int y;
 Pos(int ix, int iy) : x(ix), y(iy) {}
};
void printGrid();
int getNeighborCount(Pos cellPos);
void next(int (&universe)[SIZE][SIZE]);
void killOrBirthCells(int (&universe)[SIZE][SIZE], vector<Pos> &toKill, vector<Pos> &toBirth);
void cellFate(int &cell, Pos cellPos, int neighborCount, vector<Pos> &toKill, vector<Pos> &toBirth);
void printUniverse(int (&universe)[SIZE][SIZE]);
int main() {
 int universe[SIZE][SIZE] = {0};
 
 // glider 
 universe[0][2] = 1;
 universe[1][2] = 1;
 universe[2][2] = 1;
 universe[2][1] = 1;
 universe[1][0] = 1;
 while (1) {
 system("cls");
 next(universe);
 printUniverse(universe);
 Sleep(500);
 }
 return 0;
}
int getNeighborCount(int (&universe)[SIZE][SIZE], Pos cellPos) {
 int count = 0;
 for (int i = 0; i < 3; i++) {
 for (int j = 0; j < 3; j++) {
 int x = cellPos.x - 1 + j;
 int y = cellPos.y - 1 + i;
 // handle cells on the edge
 if (x >= 0 && y >= 0 && x < SIZE && y < SIZE
 && universe[cellPos.y - 1 + i][cellPos.x - 1 + j] == 1) {
 count++;
 }
 }
 }
 // exclude itself
 if (universe[cellPos.y][cellPos.x]) {
 return count - 1;
 }
 return count;
}
void next(int (&universe)[SIZE][SIZE]) {
 std::vector<Pos> toKill;
 std::vector<Pos> toBirth;
 
 for (int i = 0; i < SIZE; i++) {
 for (int j = 0; j < SIZE; j++) {
 Pos cellPos{j, i};
 int neighborCount = getNeighborCount(universe, cellPos);
 cellFate(universe[i][j], cellPos, neighborCount, toKill, toBirth);
 }
 }
 
 killOrBirthCells(universe, toKill, toBirth);
}
// populate toKill and toBirth vectors
void cellFate(int &cell, Pos cellPos, int neighborCount, vector<Pos> &toKill, vector<Pos> &toBirth) { 
 if (cell == 1) {
 // lonely
 if (neighborCount == 0 || neighborCount == 1) {
 toKill.push_back(cellPos);
 // overcrowded
 } else if (neighborCount >= 4 && neighborCount <= 8) {
 toKill.push_back(cellPos);
 }
 } else {
 if (neighborCount == 3) {
 toBirth.push_back(cellPos);
 }
 }
}
void killOrBirthCells(int (&universe)[SIZE][SIZE], vector<Pos> &toKill, vector<Pos> &toBirth) {
 for (Pos cellPos : toKill) {
 universe[cellPos.y][cellPos.x] = 0;
 }
 for (Pos cellPos : toBirth) {
 universe[cellPos.y][cellPos.x] = 1;
 }
 toKill.clear();
 toBirth.clear();
}
void printUniverse(int (&universe)[SIZE][SIZE]) {
 for (int i = 0; i < SIZE; i++) {
 for (int j = 0; j < SIZE; j++) {
 if (universe[i][j] == 1) {
 std::cout << 'x'; 
 } else {
 std::cout << '.'; 
 }
 }
 std::cout << '\n';
 }
}

Answer 1

If you're going to use C++, make full use of the language. Create a class to hold your universe, then all of your non-main functions can be members. You also won't have to pass around a reference to the universe array as it would be a member of the class.

One alternative to the toKill and toBirth vectors is to have two "universe" arrays. You'd use one as the source, and copy over and update the values into the second, then trade off which array you modify on the next pass thru. cellFate would return the new (updated) value for that cell, and killOrBirthCells would be eliminated.

getNeighborCount is only using i and j as offsets to the x and y values. You can redo the loops to directly update the x and y values, and also handle the edge clipping by proper tracking of the start and end values for the loops. It would also be easy to exclude the cell itself.

Combining all that we get a new getNeighborCount (that would be a member of a class):

int Universe::getNeighborCount(Pos cellPos) const {
 int count = 0;
 int sx = std::max(cellPos.x - 1, 0);
 int ex = std::min(cellPos.x + 1, SIZE - 1);
 int sy = std::max(cellPos.y - 1, 0);
 int ey - std::min(cellPos.y + 1, SIZE - 1);
 for (int y = sy; y <= ey; ++y) {
 for (int x = sx; x <= ex; ++x) {
 if (y != cellPos.y || x != cellPos.x) {
 if (universe[y][x] == 1)
 count++;
 }
 }
 }
 return count;
}

You may be able to reduce flickering by moving the system("cls"); call to immediately before the call to printUniverse. This would reduce the amount of time that you have an empty screen.

• Roland Illig: The int y looked like an obvious typo to me. But then I saw that there is no sy at all. I would have added this extra variable to not confuse human readers. The compiler is clever enough to optimize it away. Your current code is asymmetric between the x and y coordinates in a place where there is no actual asymmetry.
• 1201ProgramAlarm: @RolandIllig Good points. I've modified the code a bit to improve the readability.