Nought and Crosses (tic-tac-toe) in C++

Question

Learning coding with my daughter again. This is our first C++ program. I'm sure there's much that could be improved on. Any comments gratefully received.

New version here: Noughts & Crosses (tic-tac-toe) revisited (using C++20 modules)

ttt.cpp

#include <iostream>
#include <cstdint>
#include <limits>
#include "Board.h"

using std::cout, std::cin, std::numeric_limits, std::streamsize;

enum Player : char { HUMAN = 'X', MACHINE = 'O' };

char currentPlayer = HUMAN;
Board board;

void switchPlayer() {
    currentPlayer = (currentPlayer == HUMAN) ? MACHINE : HUMAN;
}

uint8_t get_human_move(){
    int digit;
    while (true) {
        cout << "Enter your move (1 - 9): ";
        if ((cin >> digit) && digit > 0 && digit <= 9)
            break;
        cout << "Oops... entry must be a single digit between 1 and 9\n";
        cin.clear(); 
        cin.ignore(numeric_limits<streamsize>::max(), '\n'); 
    }
    return static_cast<uint8_t>(digit);
}

void makeHumanMove(){
    uint8_t position;        
    while (true){
        position = get_human_move();
        if (board.placeSymbol(position,HUMAN)) {
            break;
        }
        cout << "Already taken, try again.\n";
    }
}

void clear_screen() {
    #ifdef _WIN32
        system("cls");
    #else 
        system("clear");
    #endif
}

int main() {
    board.display();
    while (true) {
        if (currentPlayer == HUMAN)
            makeHumanMove();
        else
            board.placeSymbol(board.getMachineMove(),MACHINE); 
        clear_screen();     
        board.display();
        if (board.win()){
            cout << currentPlayer << " wins\n";
            break;
        } 
        if (board.tie()){
            cout << "It's a tie!\n";
            break;
        }  
        switchPlayer(); 
    }
    return 0;
}

Board.h

    #include <unordered_set>
    #include <vector>
    #include <algorithm>
    #include <random>
    #include <iostream>
    
    class Board {
    
        private:
         
        char board[3][3] = {{'1','2','3'},{'4','5','6'},{'7','8','9'}};
        // set of available positions from which machine generates its next move
        std::unordered_set<uint8_t> availablePositions = {1, 2, 3, 4, 5, 6, 7, 8, 9};
    
        void removeAvailablePosition(int position);
                    
        public:
        
        Board();
        void display();
        bool placeSymbol(int position, char symbol);
        bool tie();
        bool win();
        int getMachineMove();
    };

Board.cpp

#include "Board.h"

using std::cout, std::cin;

Board::Board(){
    //shuffle the unordered set to ensure different ordering each run
    std::vector<int> vec(availablePositions.begin(), availablePositions.end());
    std::random_device rd;
    std::mt19937 gen(rd());
    std::shuffle(vec.begin(), vec.end(), gen);
    availablePositions.clear();
    for (int elem : vec) {
        availablePositions.insert(elem);
    }
}

void Board::display() {
    cout << "\n";
    cout << " " << board[0][0] << " | " << board[0][1] << " | " << board[0][2] << "\n";
    cout << "-----------\n";
    cout << " " << board[1][0] << " | " << board[1][1] << " | " << board[1][2] << "\n";
    cout << "-----------\n";
    cout << " " << board[2][0] << " | " << board[2][1] << " | " << board[2][2] << "\n";
    cout << std::flush;
} 
    
void Board::removeAvailablePosition(int position){
    availablePositions.erase(position);
}  

bool Board::placeSymbol(int position, char symbol) {
    uint8_t row = (position-1) / 3;
    uint8_t col = (position-1) % 3;
    if (board[row][col] == 'X' || board[row][col] == 'O') {
        return false;
    }
    board[row][col] = symbol;
    removeAvailablePosition(position);
    return true;
}

bool Board::win() {
    for (uint8_t n=0; n<3; n++) {
          //col n
        if (board[n][0] == board[n][1] && board[n][1] == board[n][2]) {
            return true;
        }
        // row n
        if (board[0][n] == board[1][n] && board[1][n] == board[2][n]) {
            return true;
        }
    }
    // diagonals
    if (board[0][0] == board[1][1] && board[1][1] == board[2][2]) {
        return true;
    }
    if (board[0][2] == board[1][1] && board[1][1] == board[2][0]) {
        return true;
    }
    return false;
}

bool Board::tie(){
    if (board[0][0] != '1' && board[0][1] != '2' && board[0][2] != '3' &&
        board[1][0] != '4' && board[1][1] != '5' && board[1][2] != '6' &&
        board[2][0] != '7' && board[2][1] != '8' && board[2][2] != '9') {
        return true;
    } else {
        return false;
    }
}

int Board::getMachineMove(){
     // just gets the first available position from unordered set of available positions
     // i.e. a random move!
    uint8_t move = *availablePositions.begin();
    removeAvailablePosition(move);
    return move;
}

Answer 1

For a first C++ program, this already very well written, even taking into consideration that you have already written some games in Java and Python. In particular, you have used a correct way to create a random number generator and shuffle a vector with it, you are using using correctly, and split the code into reasonably short and well-defined functions. The names of functions and variables are also clear and concise.

There are still some improvements possible:

Don't use system() to clear the screen

I've seen this in many reviews for games, where people want to clear the screen between moves. The problem with system() is that it is not portable (which you already know), very inefficient (it needs to fork a shell, which interprets the command, which then has to fork and call the actual binary that clears the screen), and potentially unsafe (a different configuration of the shell's environment might cause these shell commands to do something different than you expect). See also this StackOverflow question.

I would either use an ANSI escape code to clear the screen, although this doesn't work with all versions of Windows, or just opt to not clear the screen at all.

Not clearing will not make the game any less entertaining. After you have created a functionally working game, then you could try to make it more visually appealing. Once you can focus all your attention to that, you can go beyond just clearing the screen between moves, and start using a curses library to do fancy text "drawing" in the console, or use a GUI library.

Incorrect use of std::unordered_set

While it might seem like your std::unordered_set is working like you intended, it actually does something different, but most of the time you are "lucky" and the result is random anyway. However, there is no guarantee whatsoever that the element you first added to availablePositions is also the first one you get out when calling getMachineMove().

The name might also fool you, because you have a set of numbers and you want them in an "unordered order".

However, you just want a sequence of shuffled numbers. Consider using just the std::vector to store that sequence. For example:

class Board {
    std::vector<int> availablePositions = {1, 2, 3, 4, 5, 6, 7, 8, 9};
    …
};

Board::Board() {
    std::random_device rd;
    std::mt19937 gen(rd());
    std::shuffle(availablePositions.begin(), availablePositions.end(), gen);
}

int Board::getMachineMove(){
    uint8_t move = availablePositions.back();
    availablePositions.pop_back();
    return move;
}

Read input line by line

After trying to read a number, you call cin.clear() and cin.ignore() to get rid of potential cruft in the input. Clearly you want the user to enter one number per line. However, there is a better way, and that is to make sure you always read a whole line by using std::getline(), and then try to parse it as an integer:

uint8_t get_human_move() {
    int digit;
    while (true) {
        cout << "Enter your move (1 - 9): ";
        std::string line;
        std::getline(cin, line);
        digit = std::stoi(line);
        if (digit > 0 && digit <= 9)
            break;
        cout << "Oops... entry must be a single digit between 1 and 9\n";
    }
    return static_cast<uint8_t>(digit);
}

Handle I/O errors

One of the more boring and annoying parts of programming is dealing with errors. However, a good program handles errors as best as it can. Ignoring errors can lead to unexpected and potentially dangerous behavior.

When dealing with I/O, consider that errors can happen any time you read or write, even to and from the console. Some of them are recoverable, others are not. In the case of the get_human_move() function I showed above, if std::getline() fails, it is an unrecoverable error. However, if parsing the integer fails, you can recover by asking for a new input.

To deal with the unrecoverable error, the simplest here is to just throw an exception, which will cause the program to abort if you don't catch it. That's a perfectly good way to deal with this, since the user will probably see some error message, and the exit code from the program will be non-zero. So:

if (!getline(cin, line)) {
    throw std::runtime_error("Could not read from standard input!");
}

As for parsing integers: your cin >> digit will read up to the first non-digit character. So if you have 1O as input (a one followed by a capital o), then it will happily report it read 1. If it couldn't read any number at all, it will write nothing to digit. Since you never initialized that variable, there could be a value between 1 and 9 in there. Make sure you initialize it correctly, and/or check that cin.fail() is false after trying to read the number.

std::stoi() has similar issues, but at least it will throw and exception if it couldn't read any number, or if the number read couldn't be stored into digit. But then you should of course catch those exceptions if you want the user to be able to try to enter a number again.

Since C++17 there is std::from_chars() which can also be used to parse numbers, and which will not throw exceptions, but rather returns an error code that you have to check.

For fun, also try entering 010 as the input.

Avoid code duplication

For a 3 by 3 board game, it's not so bad to have to repeat things a few times. But what if you had a 10 by 10 board? Your code would quickly become unmaintainable. It would be a good excercise to define the board's width and height as constants, and then to only use those constants in your code instead of 0 through 9 manually. For example:

class Board {
    static constexpr std::size_t rows = 3;
    static constexpr std::size_t cols = 3;
    char board[rows][cols];
    std::vector availablePositions;
    …
};

Board::Board() {
    std::size_t position = 1;

    for (std::size_t row = 0; row < rows; ++row) {
        for (std::size_t col = 0; col < cols; ++cols) {
            board[row][col] = position;
            availablePositions.push_back(position);
            ++position;
        }
    }
    …
};              

You would have to modify a lot of the code to use loops. However, once you have made everything depend on the constants rows and cols, you can try playing noughts and crosses on different size boards very easily, by changing the constants and recompiling.

Once you can play it on a larger board, you'll probably realize quite quickly that it's much easier to get a draw. You might want to change the rules in some way. There are many variations of tic-tac-toe, and similar games like Connect Four, that you can implement with some simple changes.

Answer 2

<cstdint> declares std::uint8_t. It seems your platform also populates the global namespace, but it's not portable to depend on that.

---

Many of the Board member functions can be declared const (e.g. display(), tie(), win(), ...)