Tic Tac Toe in C++ with classes

Question

My first language is Python and I learned considerably a good amount of it. So now I started learning C++. I know the basics of C++ and about classes and inheritance.

This is my first serious project with classes. (I am thinking of adding an ai)

I wanted to know where I could -

• Optimize the performance.
• Improve my class.
• Improve my win and draw detection functions. (Sadly, I cant use ... == ... ==... == in C++)

Here is my code -

#include <iostream>

class TicTacToe
{
private:
    char board[9] = {' ', ' ', ' ', ' ', ' ', ' ', ' ', ' ', ' '};
    char current_turn = 'X';
    bool playing = true;
    int state = 0;
    int input;

public:
    void print_board();
    int play_move(int index, char move);
    int check_win(char move);
    void start();
};

int main()
{
    TicTacToe game;
    game.start();
    return 0;
}

void TicTacToe :: start()
{
    while (playing == true)
    {
        print_board();
        std::cout << "Play your move " << current_turn << std::endl;
        std::cin >> input;
        if (play_move(input, current_turn) == 0)
        {
            std::cout << "Box already occupied" << std::endl;
            continue;
        };
        state = check_win(current_turn);
        if (state == 1)
        {
            print_board();
            std::cout << current_turn << " wins the game!" << std::endl;
            break;
        }
        else if (state == 2)
        {
            std::cout << "Draw!" << std::endl;
            break;
        };
        current_turn = (current_turn == 'X') ? 'O' : 'X';
    };
};

void TicTacToe :: print_board()
{
    for (int i = 0; i < 9; i++)
    {
        if (i == 1 || i == 2 || i == 4 || i == 5 || i == 7 || i == 8)
        {
            std::cout << " | ";
        }
        std::cout << board[i];
        if (i == 2 || i == 5)
        {
            std::cout << std::endl;
            std::cout << "---------" << std::endl;
        }
    }
    std::cout << std::endl;
};

int TicTacToe :: play_move(int index, char move)
{
    if (index >= 0 && index < 9)
    {
        if (board[index] == ' ')
        {
            board[index] = move;
            return 1;
        }
    }
    return 0;
};

/*
   0 1 2
   3 4 5
   6 7 8
*/
int TicTacToe ::check_win(char move)
{
    if (
        // Horizontal checks
        (board[0] == move && board[1] == move && board[2] == move) ||
        (board[3] == move && board[4] == move && board[5] == move) ||
        (board[6] == move && board[7] == move && board[8] == move) ||
        // Vertical Checks
        (board[0] == move && board[3] == move && board[6] == move) ||
        (board[1] == move && board[4] == move && board[7] == move) ||
        (board[2] == move && board[5] == move && board[8] == move) ||
        // Diagonal Checks
        (board[0] == move && board[4] == move && board[8] == move) ||
        (board[2] == move && board[4] == move && board[6] == move))
    {
        return 1;
    }
    else
    {
        bool draw = true;
        for (int i = 0; i < 9; i++)
        {
            if (board[i] == ' ')
            {
                draw = false;
                break;
            }
        }
        if (draw == true)
        {
            return 2;
        }
    }
    return 0;
};

Answer 1

Let's look at the class first:

class TicTacToe
{
private:
    char board[9] = {' ', ' ', ' ', ' ', ' ', ' ', ' ', ' ', ' '};
    char current_turn = 'X';
    bool playing = true;
    int state = 0;
    int input;

public:
    void print_board();
    int play_move(int index, char move);
    int check_win(char move);
    void start();
};

All the state is private and the functions are all public - that seems sensible. We can omit the private: label since that's the default for class, but it does no harm to have it, and it is more explicit.

The print and check functions shouldn't need to modify the state of the object, so they probably ought to be declared const:

    void print_board() const;
    int check_win(char move) const;

The function we've called print_board() is usually spelt <<, with the signature

std::ostream& operator<<(std::ostream&, const TicTacToe&)

We need it to be a non-member function (and probably friend so it can read the innards of TicTacToe objects).

start() isn't a great name - it seems it plays a whole game. I'd call it play() or play_game().

It's not clear that input needs to be a member - usually inputs are transient values that only later translate into state changes. The integer state is a very vague name; when I went to see where it's used, it seems it doesn't need to be a member either.

I would consider changing current_turn into something like bool player_is_x - that's easier to toggle (player_is_x = !player_is_x) and still quite simple to convert to the appropriate letter (player_is_x ? 'X' : 'O' - or more obscurely, player_is_x["OX"], but really don't do that).

You might want to consider having a class to represent a board, and one that knows how to play the game (managing whose turn it is, etc.).

---

In a couple of places, we have redundant comparisons:

    while (playing == true)

    if (draw == true)

while and if accept booleans, so they can be simply:

    while (playing)

    if (draw)

We never write if ((x == y) == true), do we?

---

The structure of check_win() is

if (⋯)
{
    return 1;
}
else
{
    bool draw = true;
    for (⋯)
    {
        if (⋯)
        {
            draw = false;
            break;
        }
    }
    if (draw == true)
    {
        return 2;
    }
}
return 0;

Whenever we have if (⋯) { return; } any following else can just go directly after without the else keyword:

if (⋯)
{
    return 1;
}
bool draw = true;
for (⋯)
{
    if (⋯)
    {
        draw = false;
        break;
    }
}
if (draw == true)
{
    return 2;
}
return 0;

Now, the draw variable is only ever set to determine what to return. But we can simply return immediately instead of setting it:

if (⋯)
{
    return 1;
}
for (⋯)
{
    if (⋯)
    {
        return 0;
    }
}
return 2;

That's much simpler. Simpler still (for the caller, too) is to have separate member functions to check for a win and for a draw.

When we're checking for a win, do you see any pattern in the cell indexes here?

    // Horizontal checks
    (board[0] == move && board[1] == move && board[2] == move) ||
    (board[3] == move && board[4] == move && board[5] == move) ||
    (board[6] == move && board[7] == move && board[8] == move) ||
    // Vertical Checks
    (board[0] == move && board[3] == move && board[6] == move) ||
    (board[1] == move && board[4] == move && board[7] == move) ||
    (board[2] == move && board[5] == move && board[8] == move) ||
    // Diagonal Checks
    (board[0] == move && board[4] == move && board[8] == move) ||
    (board[2] == move && board[4] == move && board[6] == move)

Each straight-line check is an arithmetic progression, with a step of 1 for horizontal, 3 for vertical, 4 for major diagonal and 2 for minor diagonal. So we could write a small (private) helper rather than writing out all cases like that:

bool TicTacToe::check_line(int pos, int step) const
{
    char val = board[pos];
    pos += step;
    if (board[pos] != val) { return false; }
    pos += step;
    if (board[pos] != val) { return false; }
    return true;
}

bool TicTacToe::check_win() const
{
    for (int i = 0;  i < 3;  ++i) {
        // horizontal line?
        if (check_line(i*3, 1)) { return true; }
        // vertical line?
        if (check_line(i, 3)) { return true; }
    }
    // diagonals
    return check_line(0, 4) || check_line(2, 2);
}

bool TicTacToe::check_draw() const
{
    for (char c: board) {
        if (c == ' ') {
            return false;
        }
    }
    return true;
};

(Note that I didn't need to pass the current player here - if there's any line, it can only have been made in the most recent turn, or there would have been a win earlier).

And the call is just:

    if (check_win()) {
        print_board();
        std::cout << current_turn << " wins the game!\n";
        break;
    }
    if (check_draw()) {
        std::cout << "Draw!\n";
        break;
    };

---

When reading input, always check the stream state afterwards. If the user enters something unexpected, the variable won't have a useful value:

    int input;
    std::cin >> input;
    if (!std::cin) {
        std::cerr << "Input error\n";
        return;
    }

Answer 2

• Only the start() member function needs to be public. The other member functions can be private.

• Member functions that don't change any member variables in the class should be marked const, e.g. void print_board() const;

• play_move returns success or failure, so it should return a bool and not an int.

• It would be better to use an enum for state, instead of an int. That way we can use meaningful names, e.g.: enum class State { undecided, tie, winner };

• state doesn't need to be a member variable. It can be a local variable in start().

• playing could also be a local variable, but since it's never set to false we don't actually need it! The loop in start() could just be while (true) { ... }.

• Note that when testing a boolean variable like playing in a while or if condition, we should do if (playing), not if (playing == true). (It's already a boolean so we don't need the equality comparison!)

• input can also be a local variable... in fact the current_turn can also be a local variable, since we pass it everywhere we need it.

---

Rather than checking lots of magic numbers in print_board, we could iterate over rows and columns and calculate the relevant board index, e.g.:

    const int board_size = 3; // could be a static constant member variable in the class

    for (int y = 0; y != board_size; ++y)
    {
        for (int x = 0; x != board_size; ++x)
        {
            if (x != 0)
            {
                std::cout << " | ";
            }

            std::cout << board[y * board_size + x];
        }

        std::cout << std::endl;
        std::cout << "---------" << std::endl;
    }

    std::cout << std::endl;

Answer 3

in print_board:

• instead of checking against all is I would try to find a formula (probably containing a modulo) to check against once
• if you want to cout, don't put an endl at every line (it flushes the buffer to screen), use a simple '\n' and an endl or flush at the end.

in check_win: instead of again doing all these separate comparisons, I might add up the contents of the possible winning lines and compare each with 3*move

The last one of course is only viable if you can ensure that 3*token1 is neither once nor twice the value of token2 (which I haven't checked for 'X' and 'O' ...)

Draw detection:

bool draw = std::find(board, board+9, ' ') == board+9;

Answer 4

Be smart!!

Imagine it's Friday afternoon. You are feeling impressed with yourself for completing the assignment right on time and just when you are about close your little MacBook, your boss walks in and asks if you can modify your program to work on a 5 x 5 grid? Tears run down your face as you realize that your weekend is ruined. You should always code for reusability.

enum Diagonal
{
    Principle,
    Secondary
};

template <class T> class TicTacTo
{
private:
    std::size_t _rows;
    std::size_t _cols;
    T _emptySpace;
    std::vector<T> _cells = {};

public:
    TicTacTo(std::size_t rows, std::size_t cols, T _emptySpace)
      : _rows{rows},
        _cols{cols},
        _emptySpace{_emptySpace}
    {
        _cells.resize(rows * cols, _emptySpace);
    }

public:
    T& cell(std::size_t row, std::size_t col) {
        return _cells[col + row * _cols];
    }

    const std::vector<T> getCol(std::size_t col) {
        std::vector<T> v(_rows);
        for (std::size_t row = 0; row < _rows; row++)
            v[row] = cell(row, col);
        return v;
    }

    const std::vector<T> getRow(std::size_t row) {
        std::vector<T> v(_cols);
        for (std::size_t col = 0; col < _cols; col++)
            v[col] = cell(row, col);
        return v;
    }

    const std::vector<T> getDiagonal(Diagonal d) {
        std::vector<T> v;
        for (std::size_t col = 0; col < _cols; col++) {
            if (col < _rows) {
                v.push_back(cell(col, d == Diagonal::Principle ? col : _rows - col - 1));
            }
        }
        return v;
    }

    bool check_done() {
        return std::count(_cells.begin(), _cells.end(), _emptySpace) == 0;
    }

    bool check_equal(std::vector<T> v, T move) {
        return v[0] == move && std::equal(v.begin() + 1, v.end(), v.begin());
    }

    bool check_win(T move) {
        // check rows
        for (std::size_t r = 0; r < _rows; r++) {
            if (check_equal(getRow(r), move))
                return true;
        }
        // check columns
        for (std::size_t c = 0; c < _cols; c++) {
            if (check_equal(getCol(c), move))
                return true;
        }
        // check diagonals
        for (int d = 0; d < 2; d++) {
            if (check_equal(getDiagonal(d == 0 ? Diagonal::Principle : Diagonal::Secondary), move))
                return true;
        }
        return false;
    }

    void print() {
        for (std::size_t row = 0; row < _rows; row++) {
            for (std::size_t col = 0; col < _cols; col++) {
                std::cout << " " << cell(row, col);
                if (col < _cols - 1)
                    std::cout << " |";
            }
            if (row < _rows - 1) {
                std::cout << std::endl;
                for (std::size_t col = 0; col < _cols; col++) {
                    std::cout << (col < _cols - 1 ? "----" : "---");
                }
                std::cout << std::endl;
            }
        }
        std::cout << std::endl;
    };
};

int main() {
    TicTacTo<char> ttt(5, 5, ' ');
    ttt.cell(0, 4) = 'x';
    ttt.cell(1, 3) = 'x';
    ttt.cell(2, 2) = 'x';
    ttt.cell(3, 1) = 'x';
    ttt.cell(4, 0) = 'x';
    ttt.print();

    std::cout << "win = " << ttt.check_win('x') << std::endl;
}

The next edit will replace vector with valarray and use stride etc.