Conway's Game of Life class

Question

This is a work in progress, I plan to use this class with OpenGL to make simulations look like they do on the Wikipedia page.

I'm considering doing bounds checking when accessing my private member arrays but I'm not sure if it's necessary if they are not visible from the outside.

Any feedback is welcome, I want to learn as much as possible.

Species.h

#ifndef SPECIES_H
#define SPECIES_H

#include <ostream>

// Species implements Conway's Game of Life, which is a cellular automaton.
// The game is a zero-player game, meaning that its evolution is determined
// by its initial state, requiring no further input.
// The game begins with an initial configuration of cells, which then can
// be observed evolving.
// The universe of the game is a two-dimensional grid (std::array, wraps around) of
// square cells, each of which is in one of two possible states, alive or dead.
// Every cell interacts with its neighbours, the amount of these neighbours
// determines if the cell lives on or dies.
//
// The class takes an initial configuration of the NxN grid as input.
// Example:
//    std::array<std::array<Species<N>::Cell, N>, N> grid {};
//    Species<N> species_a(grid);

template <int size>
class Species {

public:

    enum class Cell {
        DEAD,
        ALIVE
    };

private:

    using grid = std::array<std::array<Cell, size>, size>;

    // All cells evolve or die simultaneously, therefore we need
    // one grid for the current generation and another grid for
    // the future/evolved generation.
    grid generation_a {};
    grid generation_b {};

    // Using pointers to each grid allows us to easily change
    // which grid is considered the current generation and which
    // is considered the future generation. We also don't need
    // to make unnecessary copies.
    grid* generation_current {&generation_a};
    grid* generation_future  {&generation_b};

    void change_state(int, int /* coordinates */, Cell);
    bool alive(int, int /* coordinates */) const;
    int  count_alive_neighbours(int, int /* coordinates */) const;

public:

    Species(const grid&);
    Species(const Species<size>&);
    Species<size>& operator=(const Species<size>&);

    void evolve();

    template <int samesize>
    friend std::ostream& operator<<(std::ostream&, const Species<samesize>&);
};

template <int size>
void Species<size>::change_state(int row, int column, Cell state) {
    (*generation_future)[row][column] = state;
}

template <int size>
bool Species<size>::alive(int row, int column) const {
    return (*generation_current)[row][column] == Cell::ALIVE;
}

template <int size>
int Species<size>::count_alive_neighbours(int row, int column) const {
    const int edge {size - 1};
    int alive_neighbours {0};

    // Check north
    if (row == 0) {
        if (alive(edge, column)) ++alive_neighbours;
    } else {
        if (alive(row - 1, column)) ++alive_neighbours;
    }

    // Check south
    if (row == edge) {
        if (alive(0, column)) ++alive_neighbours;
    } else {
        if (alive(row + 1, column)) ++alive_neighbours;
    }

    // Check west
    if (column == 0) {
        if (alive(row, edge)) ++alive_neighbours;
    } else {
        if (alive(row, column - 1)) ++alive_neighbours;
    }

    // Check east
    if (column == edge) {
        if (alive(row, 0)) ++alive_neighbours;
    } else {
        if (alive(row, column + 1)) ++alive_neighbours;
    }

    // Check north west
    if (row == 0 && column == 0) {
        if (alive(edge, edge)) ++alive_neighbours;
    } else if (row == 0) {
        if (alive(edge, column - 1)) ++alive_neighbours;
    } else if (column == 0) {
        if (alive(row - 1, edge)) ++alive_neighbours;
    } else {
        if (alive(row - 1, column - 1)) ++alive_neighbours;
    }

    // Check north east
    if (row == 0 && column == edge) {
        if (alive(edge, 0)) ++alive_neighbours;
    } else if (row == 0) {
        if (alive(edge, column + 1)) ++alive_neighbours;
    } else if (column == edge) {
        if (alive(row - 1, 0)) ++alive_neighbours;
    } else  {
        if (alive(row - 1, column + 1)) ++alive_neighbours;
    }

    // Check south west
    if (row == edge && column == 0) {
        if (alive(0, edge)) ++alive_neighbours;
    } else if (row == edge) {
        if (alive(0, column - 1)) ++alive_neighbours;
    } else if (column == 0) {
        if (alive(row + 1, edge)) ++alive_neighbours;
    } else {
        if (alive(row + 1, column - 1)) ++alive_neighbours;
    }

    // Check south east
    if (row == edge && column == edge) {
        if (alive(0, 0)) ++alive_neighbours;
    } else if (row == edge) {
        if (alive(0, column + 1)) ++alive_neighbours;
    } else if (column == edge) {
        if (alive(row + 1, 0)) ++alive_neighbours;
    } else {
        if (alive(row + 1, column + 1)) ++alive_neighbours;
    }

    return alive_neighbours;
}

template <int size>
Species<size>::Species(const grid& generation_initial):
    generation_a {generation_initial} {}

template <int size>
Species<size>::Species(const Species& other) {
    if (this != &other) {
        // We only need to copy the current generation because
        // we will overwrite the future generation anyways.
        *generation_current = *other.generation_current;
    }
}

template <int size>
Species<size>& Species<size>::operator=(const Species& other) {
    if (this != &other) {
        // We only need to copy the current generation because
        // we will overwrite the future generation anyways.
        *generation_current = *other.generation_current;
    }
    return *this;
}

// Count every cell's neighbours:
//    1. If a cell has fewer than 2 or more than 3 it dies/stays dead.
//    2. If a cell has 3 it stays alive/comes alive.
//    3. If a cell is alive and has 2 neighbours it stays alive.
template <int size>
void Species<size>::evolve() {
    for (int row {0}; row < size; ++row) {
        for (int column {0}; column < size; ++column) {
            int alive_neighbours {count_alive_neighbours(row, column)};
            if (alive_neighbours < 2 || alive_neighbours > 3) {
                change_state(row, column, Cell::DEAD);
            } else if (alive_neighbours == 3 || alive(row, column)) {
                change_state(row, column, Cell::ALIVE);
            }
        }
    }
    // Switch generations. The future generation becomes the new
    // current generation, the current generation becomes the next
    // future generation.
    grid* generation_temporary {generation_current};
    generation_current = generation_future;
    generation_future  = generation_temporary;
}

template <int size>
std::ostream& operator<<(std::ostream& out, const Species<size>& species) {
    for (const auto& row : *species.generation_current) {
        for (const auto& cell : row) {
            if (cell == Species<size>::Cell::ALIVE) {
                out << '#';
            } else {
                out << ' ';
            }
        }
        out << '\n';
    }
    return out;
}

#endif // SPECIES_H

main.cpp

#include <array>
#include <iostream>

#include "Species.h"

int main() {

    // Simple test

    const int size {10};

    std::array<std::array<Species<size>::Cell, size>, size> generation_initial {};
    generation_initial[1][1] = Species<size>::Cell::ALIVE;
    generation_initial[1][2] = Species<size>::Cell::ALIVE;
    generation_initial[2][1] = Species<size>::Cell::ALIVE;

    generation_initial[3][4] = Species<size>::Cell::ALIVE;
    generation_initial[4][3] = Species<size>::Cell::ALIVE;
    generation_initial[4][4] = Species<size>::Cell::ALIVE;

    Species<size> species_a(generation_initial);
    std::cout << species_a;

    species_a.evolve();
    std::cout << species_a;

    return 0;
}

Answer 1

Here are some tips about coding style, best practices etc.:

1. int is definitely neither the right type for your size template parameter, nor for method parameters such as row, column, etc. As the name suggests, std::size_t would be appropriate for most of the cases here, but at the very least all of these variables should be unsigned.

2. Name your parameters. Why bother with inline comments such as void change_state(int, int /* coordinates */, Cell); when just naming your parameters would to the job far better? Having unnamed parameters generally implies that these parameters are ignored, which is not what you want to convey here.

3. Relate parameters in code that are related in meaning. What I mean by this is that it's bad practice to pass two ints to represent an x- and a y-coordinate when you can also pass a std::pair<int, int> coordinate which represents the relation of those two parameters much better. Strive to express intent in your code.

4. The name Cell is not very well chosen in my opinion. I would have expected a name along the lines of CellState. Alternatively, you could just opt to remove it completely and use bool instead (taking a bool alive as a parameter conveys the meaning associated with it well enough).

5. Use std::swap instead of

   grid* generation_temporary {generation_current};
   generation_current = generation_future;
   generation_future  = generation_temporary;
   

6. Order your #includes. The generally recommended order is: first the header the current file is implementing (if any), then includes from the same project you're working on, then includes from other projects and libraries and finally standard library headers. Also, you should sort the headers alphabetically inside their groups (i.e. sort all std headers, but keep them last etc.).

7. If you had followed the advice from point 6, you would have quickly realized that Species.h is actually missing an #include <array>!

8. You interface to users of your class is meagre. The only way you provide to access the current game state is through operator<<, which is not exactly handy if I want to do something with the data other than writing its string representation. Since you have a very convenient representation of your grid already, you should at the very least offer a way to access the current grid directly.

9. You can omit return 0; from main since the compiler adds it anyway.

---

Most of the above points are relatively minor. The ones I would deem most important are, in order, point 7 (of course), point 8 and point 2. In general, your code is already pretty good, so the most important thing for you should probably be adapting more good practices. In particular, I recommend you to take a look at the C++ Core Guidelines which is (to my mind, at least) the most comprehensive C++ style and coding guide currently available.

Answer 2

If you're just trying to exercise some language features, that's fine, but I feel like there's some unneeded complexity here.

I wouldn't go so far as to say that using an enum is wrong, but if a cell is either dead or alive, that could be handled with a boolean. Dead or alive will only ever be a binary choice, and I think a boolean would be efficient any make sense to anyone without the need for an enum.

Similarly, I don't think that Size is a good candidate for a template parameter. It's adding a lot of complexity to the code, and it seems like you could just be using a constructor parameter. I get that you feel pushed to use it because you're using std::array, but maybe std::array isn't worth the hassle. You could use a std::vector and have the same performance or just use a plain-ol-array since you're not using iterators or anything that std::array was made for.