Text-based Tetris game in C++

Question

I am learning C++ and I've written a text-based classic Tetris game using OOP design. I would really appreciate it if someone could take a look at it and review the OOP design and tell me if something is particularly bad (or particularly good). I aim to make my code expressive, maintainable, with good abstraction levels.

main.cpp

#include "Cell.h"
#include "Piece.h"
#include "Board.h"
#include "Game.h"

int main() {

    Game start_game;

    return 0;
}

Game.h

#if !defined(GAME_H_)
#define GAME_H_

#include <iostream>
#include <vector>
#include <algorithm>
#include "windows.h"

#include "Cell.h"
#include "Piece.h"
#include "Board.h"

class Game {
private:
    Board board;
    Piece curr_piece;
    std::vector<Cell> final_pionts;
    int score;
    int speed;
    bool run;
public:

    Game();
    void controls();
    void running();
    bool hit_built_points_down();

    bool checked_move(enum move_direction dir);
    bool checked_rotate();

    void draw();
    void refresh_final_points();

    void ClearScreen();
    bool game_over();
    bool regame();


};


#endif

Game.cpp

#include "Game.h"

Game::Game() 
    :score{0}, speed{200}, run{true} {

    do {
        system("cls");
        board = Board(20, 20);
        running();
    } while(regame());

}


void Game::running() {
    while (!game_over()) {
        curr_piece = Piece(Cell((board.get_width()-1)/2, board.get_height(), 'O'));
        while (!hit_built_points_down()) {
            speed = 200;
            ClearScreen();
            curr_piece.fall_down();
            score += board.remove_row();
            board.refresh();
            refresh_final_points();
            draw();
            controls();
            Sleep(speed);

        }
    }
}

bool Game::game_over() {
    for (const auto &pnt : board.get_built_points())
        if (pnt.get_y() >= board.get_height() - 2) {
            return true;
        }
    return false;
}

bool Game::regame() {
    std::cout << "======Game over======" << std::endl;
    std::cout << "replay ?? (y/n) " << std::endl;
    char c{};
    bool isvalid{false};
    do {
        std::cin >> c;
        if (c == 'y')
            return true;
        else if (c == 'n')
            return false;
        else {
            std::cout << "invalid entry\n";
            isvalid = true;
        }
    } while(isvalid);
}

bool Game::hit_built_points_down() {

    for (const auto &next_piece_pnt : curr_piece.next_fall_down_body()) {
        //hit the ground
        if (next_piece_pnt.get_y() == 0) {
            board.insert_to_built_points(curr_piece.get_body());
            return true;
        }
        //hit built points
        for (const auto &built_pnt : board.get_built_points())
            if (next_piece_pnt == built_pnt) {
                board.insert_to_built_points(curr_piece.get_body());
                return true;
            }
    }

    return false;
}

bool Game::checked_move(enum move_direction dir) {
    for (const auto &next_piece_pnt : curr_piece.next_move_body(dir)) {
        if (next_piece_pnt.get_x() == 0 || next_piece_pnt.get_x() == (board.get_width() - 1))
            return false;

        for (const auto &built_pnt : board.get_built_points())
            if (built_pnt == next_piece_pnt) 
                return false;
    }

    curr_piece.move(dir);
    return true;
}

bool Game::checked_rotate() {
    for (const auto &next_piece_pnt : curr_piece.next_rotate_body()) {
        if (next_piece_pnt.get_x() == 0 || next_piece_pnt.get_x() == (board.get_width() - 1))
            return false;

        for (const auto &built_pnt : board.get_built_points())
            if (built_pnt == next_piece_pnt) 
                return false;
    }

    curr_piece.rotate();
    return true;
}

void Game::refresh_final_points() {

    final_pionts = board.get_all_points();
    for (auto &final_pnt : final_pionts)
        for (auto piece_pnt : curr_piece.get_body())
            if (final_pnt == piece_pnt)
                final_pnt = piece_pnt;
}

void Game::draw() {        
    for (int i{board.get_height() - 1}; i >= 0 ; i--) {
        for (int j{0}; j < board.get_width(); j++) {
            auto t = std::find(final_pionts.begin(), final_pionts.end(), Cell(j, i));
            std::cout << t->get_type();
        }
        std::cout << std::endl; 
    }
    std::cout << "\n Score = " << score << std::endl;
}

void Game::controls() {
    if (GetAsyncKeyState(VK_UP))
            checked_rotate();
        else if (GetAsyncKeyState(VK_DOWN))
            speed = 10;
        else if (GetAsyncKeyState(VK_RIGHT))
            checked_move(right);
        else if (GetAsyncKeyState(VK_LEFT))
            checked_move(left);
        else if (GetAsyncKeyState('P')) {
            //run = gameover();
        }
}

void Game::ClearScreen() {
    // Function which cleans the screen without flickering
    COORD cursorPosition;   cursorPosition.X = 0;   cursorPosition.Y = 0;   SetConsoleCursorPosition(GetStdHandle(STD_OUTPUT_HANDLE), cursorPosition);
}

Board.h

#if !defined(BOARD_H_)
#define BOARD_H_

#include "Piece.h"
#include <vector>
#include <algorithm>


class Board {
private:
    int width;
    int height;
    std::vector<Cell> all_points;
    std::vector<Cell> built_points;
public:
    Board(int width = 30, int height = 30);

    int get_width() {return width;}
    int get_height() {return height;}
    std::vector<Cell> get_all_points() {return all_points;}
    std::vector<Cell> get_built_points() {return built_points;}
    void set_built_points(std::vector<Cell> built_points) {this->built_points = built_points;}
    void set_border();

    void insert_to_built_points (std::vector<Cell> insert_points);
    int remove_row();
    void refresh();

};

#endif

Board.cpp

#include "Board.h"

Board::Board(int width, int height)
    :width{width}, height{height} {
        for (int i{0}; i < width; i++)
            for (int j{0}; j < height; j++)
                all_points.push_back(Cell(i, j));

        set_border();
}

void Board::refresh() {
    set_border();
    for (auto &all_pnt : all_points)
        for (auto built_pnt : built_points)
            if (all_pnt == built_pnt)
                all_pnt = built_pnt;    //seting the char of all_pnt to built pnt
}

void Board::insert_to_built_points (std::vector<Cell> insert_points) {
    built_points.insert(built_points.end(), insert_points.begin(), insert_points.end());
}

void Board::set_border() {
    for (auto &point : all_points) {
        if (point.get_x() == 0 || point.get_y() == 0 || point.get_x() == width - 1 || point.get_y() == height - 1)
            point.set_type('#');
        else
            point.set_type(' ');
    }
}

int Board::remove_row() {
    int removed_rows{0};
    int i{1};
    while(i < height) {

        int built_points_count = std::count_if(built_points.begin(), built_points.end(), [i](const Cell &point) {
           return (point.get_y() == i);
        });

         if (built_points_count == (width - 2)) {
            removed_rows++;

            // earse-remove idiom
            auto it = std::remove_if(built_points.begin(), built_points.end(), [i](Cell point) {
                                return (point.get_y() == i);
                                });
            built_points.erase(it, built_points.end());

            std::for_each(built_points.begin(), built_points.end(), [i](Cell &point) {
                if (point.get_y() > i)
                    point.move(0, -1);
            });

        } else
            i++;
    }

    return removed_rows;
}

Piece.h

#if !defined(PIECE_H_)
#define PIECE_H_

#include "Cell.h"
#include <vector>
#include <stdlib.h>
#include <time.h>

enum piece_type {
    t_piece,
    i_piece,
    o_piece,
    l_piece,
    j_piece,
    s_piece,
    z_piece
};

enum move_direction {
    right = 1,
    left = -1
};

class Piece {
private:
    enum piece_type type;
    std::vector<Cell> body;
    Cell pos;


public:
    Piece() = default;
    Piece(Cell pos);
    void fall_down();
    std::vector<Cell> next_fall_down_body();
    void move(enum move_direction dir);
    std::vector<Cell> next_move_body(enum move_direction dir);
    void rotate();
    std::vector<Cell> next_rotate_body();

    std::vector<Cell> get_body() {return body;} 
    void set_body(std::vector<Cell> body) {this->body = body;} 

};

#endif

Piece.cpp

#include "Piece.h"

Piece::Piece(Cell pos) 
    :pos{pos} {
        srand(time(0));
        type = static_cast<piece_type>(rand() % 7);

        if (type == t_piece)
            body = {pos, pos.shift_copy(1, 0), pos.shift_copy(-1, 0), pos.shift_copy(0, 1)};
        else if (type == i_piece)
            body = {pos, pos.shift_copy(0, -1), pos.shift_copy(0, 1), pos.shift_copy(0, 2)};
        else if (type == o_piece)
            body = {pos, pos.shift_copy(0, 1), pos.shift_copy(1, 0), pos.shift_copy(1, 1)};
        else if (type == l_piece)
            body = {pos, pos.shift_copy(0, 1), pos.shift_copy(0, -1), pos.shift_copy(1, -1)};
        else if (type == j_piece)
            body = {pos, pos.shift_copy(-1, 0), pos.shift_copy(0, 1), pos.shift_copy(0, 2)};
        else if (type == s_piece)
            body = {pos, pos.shift_copy(-1, 0), pos.shift_copy(0, 1), pos.shift_copy(1, 1)};
        else if (type == z_piece)
            body = {pos, pos.shift_copy(1, 0), pos.shift_copy(0, 1), pos.shift_copy(-1, 1)};
}

void Piece::fall_down() {
    pos.move(0, -1);
    for (auto &point : body)
        point.move(0, -1);
}

std::vector<Cell> Piece::next_fall_down_body() {
    std::vector<Cell> next_body;
    for (const auto &point : body)
        next_body.push_back(point.shift_copy(0, -1));
    return next_body;
}

void Piece::move(enum move_direction dir) {
    pos.move(dir, 0);
    for (auto &point : body)
        point.move(dir, 0);
}

std::vector<Cell> Piece::next_move_body(enum move_direction dir) {
    std::vector<Cell> next_body;
    for (const auto &point : body)
        next_body.push_back(point.shift_copy(dir, 0));
    return next_body;
}

void Piece::rotate() {
    for (auto &point : body) {
        int point_x = point.get_x() - pos.get_x();
        int point_y = point.get_y() - pos.get_y();
        point.set_coordinate( (-1 * point_y) + pos.get_x() , point_x + pos.get_y());
    }
}

std::vector<Cell> Piece::next_rotate_body() {
    std::vector<Cell> next_body;
    for (const auto &point : body) {
        int point_x = point.get_x() - pos.get_x();
        int point_y = point.get_y() - pos.get_y();
        next_body.push_back(Cell((-1 * point_y) + pos.get_x() , point_x + pos.get_y()));
    }
    return next_body;
}

Cell.h

#if !defined(CELL_H_)
#define CELL_H_

#include <iostream>

class Cell {
private:
    int x;
    int y;
    char type;
public:
    Cell(int x = 0, int y = 0, char type = ' ');

    int get_x() const {return x;}
    int get_y() const {return y;}
    char get_type() const {return type;}
    void set_type(char c) {type = c;}


    Cell shift_copy(int x_shift, int y_shift) const;
    void move(int x_move, int y_move);
    void set_coordinate(int x_new, int y_new);

    bool operator==(const Cell &rhs) const;
}; 

#endif

Cell.cpp

#include "Cell.h"


Cell::Cell(int x, int y, char type)
    :x{x}, y{y}, type{type} {}

 bool Cell::operator==(const Cell &rhs) const {
     return (x == rhs.get_x() && y == rhs.get_y());
 }

Cell Cell::shift_copy(int x_shift, int y_shift) const {
    return Cell(x + x_shift, y + y_shift, type);
}

void Cell::move(int x_move, int y_move) {
    x += x_move;
    y += y_move;
}

void Cell::set_coordinate(int x_new, int y_new) {
    x = x_new;
    y = y_new;
}

Answer 1

Hi Mahmoud and welcome to CodeReview!

It is great that you want to learn C++. Your project already is advanced and it is quite an achievement that you came to a working solution. As the project is large, I will more focus on overall design than on small improvements. Also, I lack a windows build machine, so I cannot run the program, though I expect it to work just fine.

General

• Only make functions public, that are really required from the outside of a class.
• Review your naming. There are quite some examples of functions where the intent is not really clear (hit_built_points_down(), refresh_final_points()). Why is isn't isvalid in Game::regame() called invalid?
• Separate the data storage, game logic and state display. If you want to exchange the user interface at one point in the future, why should you need to touch the Board storage?
• Keep constructors lightweight. You should only initialize internal variables, no logic belongs here.
• Reduce the number of includes in the headers. Only add headers that are required (because you reference a type defined in them).
• Learn about passing variables by reference (Piece::get_body()).

Game

• Do not put the loop in the constructor. Move it out of game, as it's intent is "running multiple games in a row". Same for Game::regame().
• Why do you configure a speed when you override it before the first use? Remove it for now, too many moving parts are harder to handle.
• You don't need check_move and check_rotate functions. Create a copy of the current piece, move and rotate and then check for collisions.

Board

• Consider changing the design. Why not having a vector of rows where each row consists of multiple cells? If your data-structure makes a cell available via its coordinates, what does the cell need to store in extent? Also, having a row based data-structure allows for easier deletion and prepending of rows.
• You should store width and height as a constant, it will not change during the game.

Piece

• Do not call srand(time(0)) here, it is not related to a "Piece". This should be done at the main class. Also pass the type as a parameter.
• I would not pass the Cell to the constructor, you can easily move the piece afterwards.
• As said further, remove the next_ functions and simply copy the piece and do collision checking with the copy.
• Remove set_body it is not referenced from anywhere.

Cell

• shift_copy should be split into two functions, a copy constructor and a shift (move?) function. You already have both.