5
\$\begingroup\$

This is my attempt on the Conway's "Game of Life" using C++. The code is functional but I am sure most aspects of the code might have been poorly constructed as I am fairly new at this. Please give any suggestions on how to improve my coding standards such as coding style, syntax, naming convention, error handling (although not much has been done) etc...

Points to note:

  1. I have used pointers (char** grid) to aid in dynamic memory allocation, this way the code can be easily tweaked to dynamically expand the grid if needed (I hope so).

  2. the temp array (char** cpy) is used for temporary copy and is always deallocated after each iteration (Release()).

Execute.h

#ifndef EXECUTE
#define EXECUTE

#include <iostream>
#include <iomanip>
#include "clear.h"

#define ALIVE '*'
#define DEAD  '.'

class Execute
{

    private:

    /* 2D grid array */
    char **grid;
    /* copy */
    char** cpy;
    /* grid row and column sizes */
    const int rows;
    const int cols;
    /* number of surrounding cells ALIVE*/
    int numOfCellsAlive;
    /* copy */
    void Clone(char** grid);
    /* release memory*/
    void Release(char** mem);
    /* compute cell status */
    void AliveOrDead(char **grid);
    /* run */
    void Run();

    protected:
    public:
    /* default C'Tor */
    Execute();
    /* C'Tor */
    Execute(int x, int y);
    /* copy constructor */
    Execute(const Execute& copy);
    /* default D'Tor */
    ~Execute();
    /* render */
    void RenderGrid(char **grid);

 };


    #endif // !EXECUTE

Execute.cpp

#include "Execute.h"

Execute::Execute(int x, int y) : rows(x), cols(y), numOfCellsAlive(0)
{

    if (grid == NULL)
    {
        grid = new char* [rows];

        for (int i(0); i < rows; i++)
        {
            grid[i] = new char[cols];

            for (int j(0); j < cols; j++)
            {
                grid[i][j] = DEAD;
            }
        }
    }

    /* alives cells */
    grid[20][20] = ALIVE;
    grid[21][22] = ALIVE;
    grid[22][19] = ALIVE;
    grid[22][20] = ALIVE;
    grid[22][23] = ALIVE;
    grid[22][24] = ALIVE;
    grid[22][25] = ALIVE;

    Run();
}

Execute::~Execute()
{

    Release(grid);
}

/* print grid */
void Execute::RenderGrid(char **grid)
{

    std::cout << "\n";

    for (int i(0); i < rows; i++)
    {
        std::cout << std::setw(5);

        for (int j(0); j < cols; j++)
        {
            std::cout << grid[i][j];
        }

        std::cout << "\n";
    }
}

/* compute next generation cycle */
void  Execute::AliveOrDead(char **grid)
{

    Clone(grid);

    for (int i(0); i < rows; i++)
    {
        for (int j(0); j < cols; j++)
        {
            if ((j + 1) < cols && cpy[i][j + 1] == '*')
            {
                numOfCellsAlive++;
            }

            if ((j - 1) >= 0 && cpy[i][j - 1] == '*')
            {
                numOfCellsAlive++;
            }

            if ((j - 1) >= 0 && (i - 1) >= 0 && cpy[i - 1][j - 1] == '*')
            {
                numOfCellsAlive++;
            }

            if ((i - 1) >= 0 && cpy[i - 1][j] == '*')
            {
                numOfCellsAlive++;
            }

            if ((i - 1) >= 0 && (j + 1) < cols && cpy[i - 1][j + 1] == '*')
            {
                numOfCellsAlive++;
            }

            if ((i + 1) < rows && (j - 1) >= 0 && cpy[i + 1][j - 1] == '*')
            {
                numOfCellsAlive++;
            }

            if ((i + 1) < rows && cpy[i + 1][j] == '*')
            {
                numOfCellsAlive++;
            }

            if ((i + 1) < rows && (j + 1) < cols && cpy[i + 1][j + 1] == '*')
            {
                numOfCellsAlive++;
            }

            // case 1: the cell dies
            if (numOfCellsAlive < 2 || numOfCellsAlive > 3)
            {
                grid[i][j] = DEAD;
            }
            // case 2: the cell stays same
            else if (numOfCellsAlive == 2)
            {
                grid[i][j] = cpy[i][j];
            }
            // case 3: the cell stays alive or is "born"
            else if (numOfCellsAlive == 3)
            {
                grid[i][j] = ALIVE;
            }

            numOfCellsAlive = 0;
        }
    }

    Release(cpy);
}

/* create copy of grid for comparism */
void Execute::Clone(char **grid)
{

     cpy = new char*[rows];

    for (int i(0); i < rows; i++)
    {
        cpy[i] = new char[cols];

        for (int j(0); j < cols; j++)
        {
            cpy[i][j] = grid[i][j];
        }
    }
}

/* deallocate memory from heap */
void Execute::Release(char** mem)
{

    if (mem != NULL)
    {
        for (int i(0); i < rows; i++)
        {
            delete[] mem[i];
        }

        delete[] mem;
    }
}

/* run the "Game of Life" */
void Execute::Run()
{
    do
    {
        RenderGrid(grid);
        AliveOrDead(grid);
        cursorHome(0, 0);

    } while (true);
}

clear.h

/* windows cursor positioning */
void cursorHome(int x, int y);

clear.cpp

#include "clear.h"

#ifdef _WIN32 || _WIN64

#include <windows.h>

void cursorHome(int x, int y)
{
    COORD pos = { x, y };
    HANDLE output = GetStdHandle(STD_OUTPUT_HANDLE);
    SetConsoleCursorPosition(output, pos);
}

#else

#include <unistd.h>

void clearconsole()
{
    write(1, "\E[H\E[2J", 7);
}

#endif

main.cpp

#include "Execute.h"

void main(void)
{

    /*std::cout << "+-----------------------+" << std::endl;
    std::cout << "| Conway's Game of Life |" << std::endl;
    std::cout << "+-----------------------+" << std::endl;*/

    try
    {
        const int rows = 50;
        const int cols = 70;
        Execute execute(rows, cols);
    }
    catch (std::exception &e)
    {
        std::cout << e.what() << std::endl;
    }
    catch (...)
    {
        std::cerr << "Fatal exception: "  << "\n";
    }

    system("pause");
}
\$\endgroup\$

1 Answer 1

1
\$\begingroup\$

This is a really fun way to learn stuff. Conway's game of life is really cool, and there are a lot of optimizations that can be made as you learn more.

Here are a few suggestions I have for your implementation:

Classes

Someone else here on Code Review said something I read the other day that I thought was pretty useful. In general, your classes should be nouns and the methods should be verbs. (My apologies to whomever said it - I'd credit you if I could remember who it was!) Your class is an Execute class, and it seems to me like that's breaking the problem down in a way that's not as helpful as it could be.

I'd recommend having a class which keeps track of the game state, and which has methods like updateGameState(). Maybe something like this:

class GameState {
public:
    GameState(const int rows, const int cols);
    GameState(const GameState& src);

    void SetCell(const int cellX, const int cellY);
    void UpdateGameState();
    // ... possibly more public stuff ...

private:
    char* gameBoard;
    int numRows;
    int numCols;

    GameState() {} // Don't allow a caller to create a GameState without
                   // supplying the proper information

    //... possibly more private stuff ...
};

Then your copy of the game state can be another object of the same type.

Separation of Concerns

In general, it's a good idea if you separate out the manipulation of your program's data from the display of it. I would add a method to either get the value of an individual cell, or an immutable version of the game board data so that some other method can display it. Something like this:

class GameState {
public:
    //... other stuff from above...
    int GetCell (const int x, const int y) const;
    // ... or alternately ...
    const char* GetGameBoard () const;
    // ... etc. ...
};

Types

I'd probably also make the various cell states into an actual type so you can have and use some safer type checking:

typedef enum CellState {
    Alive = `*`,
    Dead = '.'
} CellState;

The GetCell() method could return a CellState value of either Alive or Dead.

And your game board could by an array of CellState values instead of chars:

CellState* gameBoard;

This allows people reading the code to understand it more easily, and for the compiler to do additional type checking.

Readability

This section of code is very odd:

void clearconsole()
{
    write(1, "\E[H\E[2J", 7);
}

I've never seen this before and have no idea what it does. I assume that on some system it clears something, but it's basically a magical incantation at this point. I'd at least put in a comment that makes it clear what's going on and on which systems it applies.

Running the Game

I'd separate out the Execute::Run() method into a stand alone function something like this:

void RunGame (GameState& game)
{
    do
    {
        DisplayGameState(game);
        game.UpdateGameState();
        cursorHome(0, 0);
    } while (true);
}

The DisplayGameState() function might look something like this:

void DisplayGameState(const GameState& game)
{
    int rows = game.GetNumRows();
    int cols = game.GetNumCols();
    for (int y = 0; y < rows; y++)
    {
        for (int x = 0; x < cols; x++)
        {
            std::cout << (char)game.getCell(x, y);
        }
        std::cout << "\n";
    }
}
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.