Tic Tac Toe console game in c++

Question

For learning purposes i wrote a Tic Tac Toe game in object oriented manner. I have question about storing players in game class - Players(cpu and human) are derived from virtual class players and stored in vector as unique pointers to common class. Is there more elegant way to achieve this? Also, feel free to point out things i could improve or i do in wrong way.

For convenience purposes here is repo with whole project: repo

main.cpp

#include "TicTacToe.h"
#include <iostream>
int main()
{
    int playersAmount;
    std::cout << "0 - two CPU players" << std::endl
              << "1 - play with CPU" << std::endl
              << "2 - two players" << std::endl;
    std::cin >> playersAmount;
    TicTacToe::intro();
    do {
        TicTacToe game(playersAmount);
        game.clearConsole();
        game.printBoard();
        while (game.isRunning()) {
            game.step();
        };
        char input;
        std::cout << "Play again?(y/n)";
        std::cin >> input;
        if (input != 'y') {
            break;
        }
    } while (true);
}

TicTacToe.h

#pragma once
#include "Board.h"
#include "Players.h"
#include <array>
#include <memory>
#include <string>
#include <vector>
class TicTacToe {
public:
    TicTacToe(const int numberOfHumanPlayers);
    static void intro();
    static std::string getInputFromConsole();
    static void clearConsole();
    void printBoard() const;
    void step();
    bool isRunning() const;
    void terminate();

private:
    bool m_running { true };
    int m_currentPlayer { 0 };
    std::vector<std::unique_ptr<Player>> m_players;
    Board m_board;
    void nextPlayer();
    char returnPlayerSign(const int player) const;
};

TicTacToe.cpp

#include "TicTacToe.h"
#include <iostream>
#include <memory>
#include <stdlib.h>
#include <string>
#include <vector>
TicTacToe::TicTacToe(const int numberOfHumanPlayers)
{
    switch (numberOfHumanPlayers) {
    case 0:
        m_players.push_back(std::make_unique<PlayerCPU>("CPU 1"));
        m_players.push_back(std::make_unique<PlayerCPU>("CPU 2"));
        break;
    case 1:
        m_players.push_back(std::make_unique<PlayerHuman>("Player"));
        m_players.push_back(std::make_unique<PlayerCPU>("CPU"));
        break;
    case 2:
    default:
        m_players.push_back(std::make_unique<PlayerHuman>("Player 1"));
        m_players.push_back(std::make_unique<PlayerHuman>("Player 2"));
    }
}
void TicTacToe::step()
{
    std::cout << "Player: " << m_players[m_currentPlayer]->getName() << std::endl;
    int selectedField;
    while (true) {
        selectedField = m_players[m_currentPlayer]->provideField(m_board);
        if (m_board.isMoveAllowed(selectedField)) {
            break;
        } else {
            std::cout << "Invalid move" << std::endl;
        }
    }
    m_board.takeFieldOnBoard(selectedField, returnPlayerSign(m_currentPlayer));
    clearConsole();
    printBoard();
    if (m_board.isGameWon()) {
        std::cout
            << m_players[m_currentPlayer]->getName() << "(" << returnPlayerSign(m_currentPlayer) << ") won!" << std::endl;
        terminate();
        return;
    }
    if (!m_board.areThereFreeFields()) {
        std::cout << "Game ended" << std::endl;
        terminate();
        return;
    }
    nextPlayer();
}
void TicTacToe::printBoard() const
{
    m_board.printBoard();
}
char TicTacToe::returnPlayerSign(const int player) const
{
    if (player == 0) {
        return 'X';
    }
    return 'O';
}
void TicTacToe::nextPlayer()
{
    m_currentPlayer += 1;
    m_currentPlayer %= 2;
}
void TicTacToe::clearConsole()
{
    system("clear");
}
void TicTacToe::intro()
{
    std::cout << "Tic Tac Toe game" << std::endl;
    std::cout << "To make a move, enter number of field" << std::endl;
}
std::string TicTacToe::getInputFromConsole()
{
    std::string input;
    std::cin >> input;
    return input;
}
bool TicTacToe::isRunning() const
{
    return m_running;
}
void TicTacToe::terminate()
{
    m_running = false;
}

Board.h

#pragma once
#include <array>
#include <vector>
class Board {
public:
    Board();
    bool isGameWon() const;
    bool isMoveAllowed(const int field) const;
    bool areThereFreeFields() const;
    const std::vector<int>& returnAllowedIds() const;
    void takeFieldOnBoard(const int field, const char sign);
    void printBoard() const;

private:
    std::array<char, 9> m_board;
    std::vector<int> m_allowedFieldsIds;
    bool checkCol(const int) const;
    bool checkRow(const int) const;
    bool checkAllCols() const;
    bool checkAllRows() const;
    bool checkDiagonals() const;
};

Board.cpp

#include "Board.h"
#include <algorithm>
#include <iostream>

Board::Board()
{
    m_allowedFieldsIds.reserve(9);
    for (int i = 0; i < 9; i++) {
        m_board[i] = i + '0';
        m_allowedFieldsIds.push_back(i);
    };
}
const std::vector<int>& Board::returnAllowedIds() const
{
    return m_allowedFieldsIds;
}
void Board::takeFieldOnBoard(const int field, const char sign)
{
    m_board[field] = sign;
    m_allowedFieldsIds.erase(std::remove(m_allowedFieldsIds.begin(), m_allowedFieldsIds.end(), field), m_allowedFieldsIds.end());
}
bool Board::isMoveAllowed(const int field) const
{
    auto it = std::find(m_allowedFieldsIds.begin(), m_allowedFieldsIds.end(), field);
    if (it != m_allowedFieldsIds.end()) {
        return true;
    }
    return false;
}
bool Board::areThereFreeFields() const
{
    return !m_allowedFieldsIds.empty();
}
bool Board::isGameWon() const
{
    if (checkAllCols()) {
        return true;
    }
    if (checkAllRows()) {
        return true;
    }
    if (checkDiagonals()) {
        return true;
    }
    return false;
}
bool Board::checkRow(const int row) const
{
    if (m_board[row] == m_board[row + 1] && m_board[row + 1] == m_board[row + 2]) {
        return true;
    }
    return false;
}
bool Board::checkAllRows() const
{
    for (int i = 0; i < 9; i += 3) {
        if (checkRow(i)) {
            return true;
        }
    }
    return false;
}
bool Board::checkCol(const int col) const
{
    if (m_board[col] == m_board[col + 3] && m_board[col + 3] == m_board[col + 6]) {
        return true;
    }
    return false;
}
bool Board::checkAllCols() const
{
    for (int i = 0; i < 3; i++) {
        if (checkCol(i)) {
            return true;
        }
    }
    return false;
}
bool Board::checkDiagonals() const
{
    if (m_board[0] == m_board[4] && m_board[4] == m_board[8]) {
        return true;
    }
    if (m_board[2] == m_board[4] && m_board[4] == m_board[6]) {
        return true;
    }
    return false;
}
void Board::printBoard() const
{
    for (int i = 0; i < 9; i += 3) {
        std::cout << m_board[i] << '|' << m_board[i + 1] << '|' << m_board[i + 2] << std::endl;
        if (i < 6) {
            std::cout << "_____" << std::endl;
        }
    }
    std::cout << std::endl;
}

Players.h

#pragma once
#include "Board.h"
#include <array>
#include <string>
class Player {
public:
    Player(const std::string& name)
        : m_name(name) {};
    virtual int provideField(const Board& board) const = 0;
    const std::string& getName() const;

private:
    const std::string m_name;
};
// Human player
class PlayerHuman : public Player {
public:
    PlayerHuman(const std::string& name)
        : Player(name) {};
    int provideField(const Board& board) const override;

private:
    int askForInput() const;
};
// CPU Player
class PlayerCPU : public Player {
public:
    PlayerCPU(const std::string& name)
        : Player(name) {};
    int provideField(const Board& board) const override;
    int returnFirstAllowedField(const Board& board) const;
    int returnRandomField(const Board& board) const;
};

Players.cpp

#include "Players.h"
#include <algorithm>
#include <array>
#include <chrono>
#include <iostream>
#include <iterator>
#include <random>
#include <thread>
#include <vector>
// Player
const std::string& Player::getName() const
{
    return m_name;
}
// PlayerHuman
int PlayerHuman::provideField(const Board& board) const
{
    return askForInput();
}
int PlayerHuman::askForInput() const
{
    int field;
    std::cout << "Field #";
    std::cin >> field;
    return field;
}
// PlayerCPU
int PlayerCPU::provideField(const Board& board) const
{
    using namespace std::chrono_literals;
    std::this_thread::sleep_for(1000ms);
    return returnRandomField(board);
    return 0;
}
int PlayerCPU::returnFirstAllowedField(const Board& board) const
{
    return board.returnAllowedIds()[0];
}
int PlayerCPU::returnRandomField(const Board& board) const
{

    static std::random_device rd;
    static std::mt19937 gen(rd());
    std::uniform_int_distribution<> distr(0, board.returnAllowedIds().size() - 1);
    return board.returnAllowedIds()[distr(gen)];
}

Answer 1

This is quite good! I see some thought has gone into organizing things into classes, you thought about const parameters and functions, passing by reference, #included local headers before the standard ones, smart pointers, and you use C++'s random number generators correctly.

Use '\n' instead of std::endl

Prefer using '\n' instead of std::endl; the latter is equivalent to the former, but also forces the output to be flushed, which is usually unnecessary and might hurt performance.

Avoid using system()

Using system() has a huge overhead: it starts a shell, parses the command you give it, and then starts another process that runs /usr/bin/clear. It is also non-portable; on Windows you would have to call cls instead of clear. Finally, it is unsafe; depending on the user's environment and the way their shell has been configured, clear might not do what you expect it to do.

Consider that /usr/bin/clear is written in C, you should be able to write C++ code yourself that clears the screen without having to call another program. On most operating systems, including the latest versions of Windows, you can just print an ANSI escape code. If you want to be even more portable, you can consider using a curses library. You could also consider not bothering clearing the screen; users will see past versions of the board, but it doesn't impact the game itself.

Reading input

When you are reading input from std::cin, you have to consider a few things. First, the user might enter things that are not valid. For example, when asking how many players there are, the user could enter "-1", "3", "two", and so on. If a valid integer is read, you only check if that integer is 0 or 1, and anything else you treat as if it was 2, which might result in unexpected behavior.

Also consider that entering "two" will result in playersAmount being equal to 0, and will start a two CPU player game. Again, that is unexpected.

Finally, there could be an error reading anything from std::cin at all. For example, the terminal might be closed, or the input was redirected to something that doesn't allow reading, and so on. Again, ignoring this will result in unexpected behavior of your program.

The correct thing to do is to check whether the input was read correctly. You can check after reading something if std::cin.good() is true. There are other operations you can use to find out if it was an error reading anything at all, or whether it couldn't convert the input to the desired type. In the latter case, you could consider asking the user to try again. In the case of an error you cannot recover from, just print an error message and terminate the program.

Even if reading was succesful, make sure that the value that was read is valid for your program; if not you probably should ask the player to try again.

Reading one line at a time

You probably intend the player to write something and then press enter. That means they enter one line at a time. However, when you read somehting with std::cin >> variable, it doesn't read a whole line, instead it only reads one character, integer or word (depending on whether variable is a char, int or std::string). This can result into unexpected behavior if the user enters multiple things on one line.

Consider reading in a whole line at a time into a std::string, and then parse the string. You can do this with std::getline(). For example:

int playersAmount;

while (true) {
    std::string line;

    if (!std::getline(std::cin, line)) {
        std::cerr << "Error reading input!\n";
        return EXIT_FAILURE;
    }

    playersAmount = std::stoi(line);

    if (playersAmount >= 0 && playersAmount <= 2) {
        break;
    }

    std::cerr << "Enter a number between 0 and 2!\n";
}

Storing the set of allowed fields

You use a std::vector<int> to store the IDs of allowed fields. While this works, an even better way would be to use a std::bitset<9>.

Tracking the current player

You have the variable m_currentPlayer to track whose turn it is. However, that information is actually also available in m_board.m_allowedFieldsIds: consider that if the number of allowed fields to place a mark in is odd, it's the first player's turn, and if it is even it is the second player's turn. After making a move, you'd have removed an ID from m_allowedFieldsIds, so it will already be updated to reflect that it is the next player's move.

Not all functions need to be in a class

In C++ you can have free functions that are not part of a class. One example is main() of course. Since clearing a console is not something that is specific to a tic-tac-toe game, I would move clearConsole() out of class TicTacToe, and make it a free function. You could put it in a separate file that contains console-related functions. getInputFromConsole() could also be put there.

Storing players

Players(cpu and human) are derived from virtual class players and stored in vector as unique pointers to common class. Is there more elegant way to achieve this?

This is the right way to do it when using inheritance, which is perfectly fine in this case.

Another way to do it would be to not use inheritance, but to store a vector of std::variants:

class PlayerHuman {…}; // no inheritance from Player
class PlayerCPU {…};

using Player = std::variant<PlayerHuman, PlayerCPU>;

class TicTacToe {
    …
    std::vector<Player> m_players;
    …
};

TicTacToe::TicTacToe(const int numberOfHumanPlayers)
{
    …
    m_players.push_back(PlayerHuman("Player"));
    m_players.push_back(PlayerCPU("CPU"));
    …
}

That looks much more elegant, and no pointers are needed. However, accessing the players is not so simple. You cannot write:

std::cout << "Player: " << m_players[m_currentPlayer].getName() << '\n';

Instead, you would have to use std::visit():

std::cout << "Player: "
          << std::visit([](auto& player) { return player.getName(); },
                        m_players[m_currentPlayer])
          << '\n';

Having to write a visitor every time you need to do something with a player is going to be cumbersome and thus inelegant. However, in your code you actually only need to call std::visit() once:

void TicTacToe::step()
{
    std::visit([&](auto& player) {
        std::cout << "Player: " << player.getName() << '\n';
        // All the rest of step() here
        …
    }, m_players[m_currentPlayer]);
}

Is this more elegant? You can decide for yourself. I think it doesn't matter much for this game. However, each way has its own pros and cons, and which one to choose will depend on the situation.