Tetris clone in C++ using ncurses

Recently, just for fun, I've created a clone of the widely popular game Tetris using C++. Since I am still a novice when it comes to C++, I would really appreciate all the feedback I can get from those with more experience.

The code below is also on GitHub.

Main.cpp

#include "Game.h"
#include 
#include 
int main() {
    Game game;
    setlocale(LC_ALL, "");
    initscr();
    start_color();

    init_pair(0, COLOR_GREEN, COLOR_BLACK);
    init_pair(1, COLOR_RED, COLOR_BLACK);
    init_pair(2, COLOR_BLUE, COLOR_BLACK);
    init_pair(3, COLOR_YELLOW, COLOR_BLACK);
    init_pair(4, COLOR_GREEN, COLOR_BLACK);

    curs_set(FALSE);
    raw();
    noecho();
    nodelay(stdscr, TRUE);

    game.matrix_init();

    while (!game.isGameOver()) {
        bool can_create_block = false;
        can_create_block = game.get_last_block().move_down();
        if (can_create_block) {
            game.destroy();
            game.create_block();
        }
        game.controls();
        napms(game.getSpeed());
        if (game.getSpeed() < DEFAULT_SPEED)
            game.setSpeed(DEFAULT_SPEED);
        game.draw();
        game.gameOverChecker();
    }
    endwin();
    return 0;
}

cCoord.h

#ifndef TETRIS_CCOORD_H
#define TETRIS_CCOORD_H

#define MAX_COORDINATES  4

class cCoord {
private:
    int x,
        y;
public:
        // Getter functions
    int get_x() const;
    int get_y() const;

    // Setter functions
    cCoord set_x(int a);
    cCoord set_y(int b);

    cCoord(int a, int b) : x(a), y(b) {};
    cCoord() = default;
    ~cCoord() = default;
};

#endif //TETRIS_CCOORD_H

cCoord.cpp

#include "cCoord.h"

int cCoord::get_x() const {
    return x;
}

int cCoord::get_y() const {
    return y;
}

cCoord cCoord::set_y(int b) {
    y = b;
    return *this;
}

cCoord cCoord::set_x(int a) {
    x = a;
    return *this;
}

Block.h

```
#ifndef TETRIS_BLOCK_H
#define TETRIS_BLOCK_H

#include "cCoord.h"

class Block {
private:
cCoord coord;
public:

I see a number of things that may help you improve your code.

Hide implementation details

It was surprising to me to find so many lines of more or less raw curses code in main. The Model-View-Controller design pattern is often useful for programs like this. The model is the internal state of the game that's mostly already within your Game class, the view is currently split between main and various parts of the Game class and the controller is essentially just the game.controls() function. Separating the I/O from the game logic will help you write cleaner code and also assist if you were to decide to port the game to some other platform.

Understand random

The get_next_block member function of Game is currently this:

int Game::get_next_block() {
    int val;
    while (true) {
        std::random_device generator;
        std::uniform_int_distribution distribution(0,4);

        if((val = distribution(generator)) != prev_block)
            return val;
    }
}

There are a few problems with this. First, and most importantly, you should generally not use the std::random_device except to seed another random number generator such as mt19937. The reason for this is that std::random_device is sometimes very slow and sometimes hardware based. It tends to slow down a lot if the underlying entropy of the device is low. The second problem is that the generator should probably be static so that the distribution is pulling from the same random number generator every time instead of creating a new one. I'd rewrite it like this:

int Game::get_next_block() {
    static std::mt19937 generator(std::random_device{}());
    std::uniform_int_distribution distribution(0,4);
    int val;
    for (val = distribution(generator); val == prev_block; val = distribution(generator)) 
    { }
    return val;
}

Prefer const variables to #define

Since you're using C++, there is little reason to use #define to define a numerical constant. Instead of this:

#define DEFAULT_SPEED 300

Use this:

constexpr int default_speed{300};

Note that I've also changed that from all capital letters (which is the convention for macros) to a regular variable name according to whichever convention you're using.

Use const where practical

There are a number of places in the code where variables could be declared const such as in Game.h:

static const cCoord struct_coords[][MAX_COORDINATES + 1];
static const cCoord struct_origins[MAX_COORDINATES + 1];

Avoid the use of global variables

I see that s (which is a poor name, by the way) is a global variable. It's generally better to explicitly pass variables your function will need rather than using the vague implicit linkage of a global variable. In this case, I think it would make more sense to have this be the Game object and have a separate Screen object as the Model and View classes of a Model-View-Controller.

Write member initializers in declaration order

The Structure class has this constructor

Structure::Structure(const Structure &s) : struct_type(s.struct_type), origin(s.origin), coords(s.coords), color(s.color) {}

That looks fine, but in fact, coords will be initialized after color because members are always initialized in declaration order and color is declared before coords in this class. To avoid misleading another programmer, you should swap the order of those such that it says instead:

Structure::Structure(const Structure &s) : 
    struct_type(s.struct_type), 
    origin(s.origin), 
    color(s.color),
    coords(s.coords)
{}

This way the initialization actually proceeds from left to right as one might expect at first glance.

Be careful with signed and unsigned

In several cases, the code compares an int i with an unsigned std::size_t coords.size(). It would be better to declare i to also be std::size_t.

Pass object references where needed

It doesn't really make much sense for the collision detector functions to be static since they need the current game state to actually operate correctly. Instead, make them regular member functions and then pass a reference to the Game object for all of the various Structure functions that call one of the collision functions. Doing so will help you eliminate the ugly global variable as mentioned above.

Rethink your classes

The cCoord class is not doing anything except cluttering the code. This isn't Java, and the "getters and setters" idiom used there is not generally acceptable in modern C++. Instead. your cCoord class could simply be a simple struct since anything can set or read it anyway. Similarly, the Block class is also doing very little and doesn't benefit containing a cCoord -- it could be much simpler to have an x and y directly as part of the Block class. Also, it would make more sense if the Block class actually represented a Block (with the associated coordinates and or

StackExchange Code Review Q#150131, answer score: 2