Text-based Tetris game

How can I improve this game?

```
#include
#include
#include
#include
#include

struct Random
{
Random(int min, int max)
: mUniformDistribution(min, max)
{}

int operator()()
{
return mUniformDistribution(mEngine);
}

std::default_random_engine mEngine{ std::random_device()() };
std::uniform_int_distribution mUniformDistribution;
};

std::vector> stage(22, std::vector(13, 0));
std::vector> block =
{
{ 0, 0, 0, 0 },
{ 0, 0, 0, 0 },
{ 0, 0, 0, 0 },
{ 0, 0, 0, 0 }
};

std::vector> field(22, std::vector(13, 0));
// coordinate
int y = 0;
int x = 4;
bool gameover = false;
size_t GAMESPEED = 20000;

Random getRandom{ 0, 6 };

std::vector>> block_list =
{
{
{ 0, 1, 0, 0 },
{ 0, 1, 0, 0 },
{ 0, 1, 0, 0 },
{ 0, 1, 0, 0 }
},
{
{ 0, 0, 0, 0 },
{ 0, 1, 1, 0 },
{ 0, 1, 0, 0 },
{ 0, 1, 0, 0 }
},
{
{ 0, 0, 1, 0 },
{ 0, 1, 1, 0 },
{ 0, 1, 0, 0 },
{ 0, 0, 0, 0 }
},
{
{ 0, 1, 0, 0 },
{ 0, 1, 1, 0 },
{ 0, 0, 1, 0 },
{ 0, 0, 0, 0 }
},
{
{ 0, 0, 0, 0 },
{ 0, 1, 0, 0 },
{ 1, 1, 1, 0 },
{ 0, 0, 0, 0 }
},
{
{ 0, 0, 0, 0 },
{ 0, 1, 1, 0 },
{ 0, 1, 1, 0 },
{ 0, 0, 0, 0 }
},
{
{ 0, 0, 0, 0 },
{ 0, 1, 1, 0 },
{ 0, 0, 1, 0 },
{ 0, 0, 1, 0 }
}
};

int menu();
int gameOver();
void title();
void gameLoop();
void display();
bool makeBlocks();
void initGame();
void moveBlock(int, int);
void collidable();
bool isCollide(int, int);
void userInput();
bool rotateBolck();
void spwanBlock();

int main()
{
switch (menu())
{
case 1:
gameLoop();
break;
case 2:
return 0;
case 0:
std::cerr > a;
return 0;
}

void gameLoop()
{
size_t time = 0;
initGame();

while (!gameover)

I see a number of things that can help you improve this code.

Don't use system("cls")

There are two reasons not to use system("cls") or system("pause"). The first is that it is not portable to other operating systems which you may or may not care about now. The second is that it's a security hole, which you absolutely must care about. Specifically, if some program is defined and named cls or pause, your program will execute that program instead of what you intend, and that other program could be anything. First, isolate these into a separate functions cls() and pause() and then modify your code to call those functions instead of system. Then rewrite the contents of those functions to do what you want using C++. For example, if your terminal supports ANSI Escape sequences, you could use this:

void cls()
{
    std::cout << "\x1b[2J";
}

Isolate platform-specific code

In this code, there are several things that are DOS/Windows only including #include and the getch() and kbhit() functions within that, and also system("cls"); that I've already mentioned. Your code runs successfully on Linux if I supply those missing functions, but it would be nice if there were an #ifdef WINDOWS already in the code so that one could recompile without having to alter the source code.

Fix spelling errors

The code has spwanBlock() instead of spawnBlock() and rotateBolck() instead of rotateBlock(). These kinds of typos don't bother the compiler at all, but they will bother human readers of the code and make it a little more difficult to understand and maintain.

Use more objects

The game is written much more in the procedural style of C rather than in the object-oriented style of C++. The game itself could be an object, with most of the procedures as functions of that object. This would reduce coupling and make the program easier to understand. Also, each of the blocks could quite obviously be an object. It would also eliminate the global variables that now occupy the code, such as x, y, and gameover.

Reduce variable scope as much as possible

Almost all of the global variables can be eliminated by using objects, but if any remain, they should be static to limit them to file scope, or even better, as noted by @glampert in a comment, use an anonymous namespace.

Use string concatenation

The gameOver() and title() functions both have many repeated lines where the ostream operator<< is used multiple times with std::cout and a fixed string. Those multiple calls don't need to happen. You could simply rely on the fact that C++ merges separate constant strings automatically. For example, here is a recoded gameOver():

void gameOver()
{
    std::cout > a;
}

Note a few simple things here. First, there's only a single input and a single output call, so the using namespace std; didn't seem worthwhile and was removed. Second, the dummy variable a was declared just before use instead of at the beginning of the function. Third, because the return variable was neither useful nor used, it has been omitted.

Eliminate "magic numbers"

The code, including the implementation of initGame() is full of "magic numbers" -- that is raw numbers in the text that don't have obvious meaning. For example:

void initGame()
{
    for (size_t i = 0; i <= 20; i++)
    {
        for (size_t j = 0; j <= 11; j++)
        {
            if ((j == 0) || (j == 11) || (i == 20)) 
            {
                field[i][j] = stage[i][j] = 9;
            }

It's not at all apparent what the meaning is of 20 or 11 or 9 in this code. Meaningfully named constants would be a better way to do this.

Use const where practical

Variables such as GAMESPEED are never altered by the program and should therefore be declared const.

Use better timekeeping

The current gameLoop() routine uses a very crude increment loop to do timing. This would be much better implemented using something from std::chrono or better, the whole thing could be rewritten using an asynchronous programming model instead of a synchronous scheme.

Enhance the gameplay

The original Tetris game would delete a row once it became completely filled in, but this code doesn't do that. It also kept score and the original game was in color. You could add all of these things, the first two by making functional changes to the code, and the last item by means of the previously mentioned ANSI Escape sequences. It would make for a more interesting game.

StackExchange Code Review Q#74293, answer score: 12