Conway's game of life in C++ with SDL

This is my attempt at a basic implementation of GOL. The user can add and remove cells with the mouse and some basic stats are printed to console. Any feedback is welcome, but I have a couple of specific questions:

• In Cell::get_neighbours() it seems a bit inefficient to keep constructing vectors for each cell. In Python I would use yield to avoid this; is there anything comparable in C++?

• In InputHandler::get_input I just call three 'subfunctions' which aren't reused anywhere else. Is this good practice or is one long function better?

Below is the code in rough order of importance; alternatively here is a github link.

main.cpp is the main loop for the program; including a framerate cap:

main.cpp

#include 

#include 

#include "graphics.h"
#include "input.h"
#include "logic.h"

const int TARGET_FRAMERATE = 60;
const int TARGET_TICKRATE = 2;
const int FRAMES_PER_TICK = TARGET_FRAMERATE / TARGET_TICKRATE;
const int TARGET_TIMESTEP = 1000 / TARGET_FRAMERATE;

int main() {
    Graphics graphics;
    InputHandler input_handler;
    Gamestate gamestate;

    unsigned int timestep, time0, time1 = 0;
    unsigned int frame_count = 0;
    while (!input_handler.quit) {
        // Get time
        time0 = time1;
        time1 = SDL_GetTicks();
        timestep = time1 - time0;
        frame_count++;
        // Main game loop
        if (frame_count % FRAMES_PER_TICK == 0) {
            gamestate.update();
        }
        input_handler.get_input(gamestate);
        graphics.render(gamestate, input_handler);
        // FPS cap
        if (timestep < TARGET_TIMESTEP) {
            SDL_Delay(TARGET_TIMESTEP - timestep);
            time1 += TARGET_TIMESTEP - timestep;
        }
    }
    printf("Exiting game\n");
}

The rules and implementation of the game are in logic.cpp and its header. I tried to keep this independent of SDL and the UI portions of the code:

logic.h

```
#ifndef _logic_h_included_
#define _logic_h_included_

#include
#include

st

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

Avoid object creation

The SDL library does a lot of memory allocation and deallocation which tends to slow things down a bit. You can avoid that problem by using references in several of your "range for" loops. For example, in Gamestate::update() you could use this:

// Count neighbours of each cell
for (const Cell &c : board) {
    for (Cell &n : c.get_neighbours()) {

By using references, you avoid the Cell construction/deletion that would otherwise occur.

Reuse objects where practical

Instead of creating a new Cell inside InputHandler::get_input_mouse each time, why not just update the one that already exists? For convenience, you could create this Cell member function:

void setPos(int xval, int yval) { x=xval; y=yval; }

And alter the line within get_input_mouse to use it instead of doing yet another Cell creation/destruction cycle:

highlighted_cell.setPos((int)floor((mouse_pos_x + camera_x) / zoom),
                    (int)floor((mouse_pos_y + camera_y) / zoom));

Rethink get_neighbours

You're right to question whether the current method is efficient. It's not particularly, because it requires the creation and destruction of many objects (the std::vector and each of the Cells within it) each time it's called. This is also true of the num_neighbors map.

A small improvement might be to define a neighbours array which can be constructed once and reused:

static const std::array neighbours{{ 
    {-1, -1}, { 0, -1}, {+1, -1},
    {-1,  0}, { 0,  0}, {+1,  0},
    {-1, +1}, { 0, +1}, {+1, +1} 
}};

Then the cell counting would look like this:

for (Cell n : neighbours) {
    n += c;
    if (!num_neighbours.count(n)) {
        num_neighbours.emplace(n, 0);
    }
    ++num_neighbours[n];    // Note each cell counts itself as a neighbour
}

In this version, it still creates a new Cell for each iteration (note that this version of the loop is not using a reference) but at least the std::vector is not being created/destroyed each call.

The map could also be made static and then simply clear() it each iteration instead of incurring the construct/destroy overhead.

Provide a "zoom extents" function

In the case of a prolific colony, it may not be easy to see the whole thing on screen. One feature enhancement might be to calculate a bounding box for all cells in the board and zoom to that if some particular key is pressed.

Change the name of the window

I was somewhat surprised to see this window titled "dungeon"! I assume that was a cut-and-paste from some other program, but it probably should be changed.

StackExchange Code Review Q#131958, answer score: 6