Game of Life class

I'm a beginner in C++, and I'd like you to review the class I wrote for the Game Of Life.

GameOfLife.h

#include 
#include 
using namespace std;

class GameOfLife{

    public:
        GameOfLife(void);
        GameOfLife(const unsigned &, const unsigned &);
        ~GameOfLife(void);
        void display(void);
        void swapCell(const unsigned &,const unsigned &);
        void updateField();

    private:
        unsigned Xmax, Ymax;
        unsigned XSize, YSize;
        vector > field;
        unsigned newValCell(const unsigned &, const unsigned &);
};

GameOfLife.cpp

```
#include "GameOfLife.h"

GameOfLife::GameOfLife(void) {
Xmax = 10;
Ymax = 10;
XSize = Xmax + 2;
YSize = Ymax + 2;
for (unsigned i = 0; i != XSize; i++)
{
vector inter;
for (unsigned j = 0; j != YSize; j++)
{
inter.push_back(0);
}
field.push_back(inter);
}
}

GameOfLife::GameOfLife(const unsigned &x, const unsigned &y) {
Xmax = x;
Ymax = y;
XSize = Xmax + 2;
YSize = Ymax + 2;
for (unsigned i = 0; i != XSize; i++)
{
vector inter;
for (unsigned j = 0; j != YSize; j++)
{
inter.push_back(0);
}
field.push_back(inter);
}
}

GameOfLife::~GameOfLife(void)
{
return;
}

void GameOfLife::swapCell(const unsigned &x, const unsigned &y)
{
field[x][y] = !field[x][y];
return;
}

// return the new value of a cell depending on the surrounding values
unsigned GameOfLife::newValCell(const unsigned &x, const unsigned &y)
{
unsigned stok;
stok = field[x-1][y-1]
+ field[x-1][y]
+ field[x-1][y+1]
+ field[x][y-1]
+ field[x][y+1]
+ field[x+1][y-1]
+ field[x+1][y]
+ field[x+1][y+1];

switch(stok)
{
case 3: return 1;
case 2: return field[x][y];
default: return 0;
}
}

void GameOfLife::updateField(void)
{
vector > stockField = field;

Here are some things that may help you improve your code:

Don't abuse using namespace std

Putting using namespace std at the top of every program is a bad habit that you'd do well to avoid. It is particularly bad to put it into a header file, so please don't do that.

Eliminate unused variables

The XSize and YSize variables are only used once in the constructor and can easily be eliminated.

Prefer modern initializers for constructors

The constructors can be collapsed to a single one with default values and a parameter intialization style. The declaration can have the default 10 x 10 size:

GameOfLife(const unsigned &x=10, const unsigned &=10);

The implementation can be much simplified by using std::vector constructors that take a count, value pair:

GameOfLife::GameOfLife(const unsigned &x, const unsigned &y) :
    Xmax(x), Ymax(y), field(Xmax+2, vector(Ymax+2, 0))
{
}

Consider using bool instead of int for field

On my machine, the current code allocates a total of 3276 bytes, but that is reduced to 2600 if the type of field values is changed from int to bool.

Reconsider function and field names

The name swapCell would probably be better named toggleCell because it's not really swapping with anything in the sense of std::swap. Also, the naming of Xmax and XSize with lowercase 'm' and uppercase 'S' respectively seems a little inconsistent.

Let the compiler create default destructor

The compiler will create a destructor by default which is essentially identical to what you've got, so you can simply omit both the declaraton and implementation from your code.

Omit return 0

When a C++ program reaches the end of main the compiler will automatically generate code to return 0, so there is no reason to put return 0; explicitly at the end of main. Similarly, there's no good reason to put return; at the end of a void function.

Be careful with signed and unsigned

In the current display routine, the loop integers i and j are both signed int values, but they're being compared with unsigned quantities Xmax and Ymax. Better would be to declare them all as unsigned or perhaps size_t.

Use const where practical

The current display() routine does not (and should not) modify the underlying object, and so it should be declared const:

void display(void) const;

Less obviously, perhaps, the newValCell() helper function should also be const.

Use an ostream &operator<< instead of display

The current code has void GameOfLife::display(void) but what would make more sense and be more general purpose would be to overload an ostream operator<< instead. The body of the existing display routine would only need small modifications:

friend std::ostream &operator<<(std::ostream &out, const GameOfLife &game)
{
    for (unsigned i = 1; i != game.Xmax + 1; i++)
    {
        for (unsigned j = 1; j != game.Ymax + 1; j++)
        {
            out << game.field[i][j] << " ";
        }
        out << "\n";
    }
    return out;
}

Then the main routine could use this:

cout << a << "-------\n";

instead of this:

a.display();
cout << "-------\n";

Reconsider the interface

Each call to updateField is something of an increment, so one possibility would be to declare it as an increment operator:

GameOfLife &operator++() { updateField(); return *this; }

If you did so and used the previous suggestion, the last part of the code for main could look like this:

cout << a << "-------\n";
for (int i=0; i < 5; ++i) 
{
    cout << ++a << "-------\n";
}

StackExchange Code Review Q#78212, answer score: 7