Tic-tac-toe game with Board class and enum for players

Question

I've been learning C++ on my own and working solo on trying to implement some of the concepts I've been reading.

I'm wondering if this is the correct group to get someone to point me to other info regarding the code I've wrote or if I'm getting the concepts right.

I've tried to build Tic Tac Toe using a class system. I've implemented 2 ENUMS: PLAYERto manage the current player and populate the board with data. and also BOARDSTATUS to ease of access to the current state of the board internal for the Board Class.

I've created a STRUCT: BoardMove that contains all the information needed to place a move in the board. This way I can use it via the AI Class that has it's on Board for recursivelycheck the possible moves and also it can be accessed by the Game itself to submit player moves.

Here is the Board class header file:

#define ROWS 3
#define COLS 3
#define CHECKS 3

enum class PLAYER { BLANK = ' ', PLAYER1 = 'X', PLAYER2 = 'O' };

struct BoardMove
{
    int row;
    int col;
    PLAYER playerTurn;
};


class Board
{
public:
    enum  class BOARDSTATUS { CLEAR, WINPLAYER1, WINPLAYER2, DRAW };

    Board();
    ~Board();

    void PrintBoard();
    bool InsertMove(BoardMove newMove);
    inline BOARDSTATUS GetBoardStatus() { return this->boardState; }
    
    PLAYER board[ROWS][COLS];

private: 

    BOARDSTATUS boardState;
    void ClearBoard();
    void CheckStatus(PLAYER player);
    bool AllFieldsTheSame(unsigned int startRows, unsigned int startCols, unsigned int deltaRow, unsigned int deltaCol);
    unsigned int numberOfPlays = 0;
    
    
    

};

Here is the Board Class implementation:

Board::Board()
{
    this->ClearBoard();

}

Board::~Board()
{}


//
// Foud good solution here:
// https://stackoverflow.com/questions/1056316/algorithm-for-determining-tic-tac-toe-game-over
//

void Board::CheckStatus(PLAYER player)
{


    bool win = false;

    for(int i = 0; i < ROWS; i++)
    {
        if(AllFieldsTheSame(i, 0, 1, 0))
            win = true;
            
    }
    for(int i = 0; i < COLS; i++)
    {
        if(AllFieldsTheSame(0, i, 0, 1))
            win = true;
    }
    if(AllFieldsTheSame(0, 0, 1,1))
        win = true;
    if(AllFieldsTheSame(2, 0, -1, 1))
        win = true;

    if(win == true)
        this->boardState = (player == PLAYER::PLAYER1) ? BOARDSTATUS::WINPLAYER1 : BOARDSTATUS::WINPLAYER2;

    else if(this->numberOfPlays == 8)
        this->boardState = BOARDSTATUS::DRAW;
}


//
// Foud good solution here:
// https://stackoverflow.com/questions/1056316/algorithm-for-determining-tic-tac-toe-game-over
//

bool Board::AllFieldsTheSame(unsigned int startRows, unsigned int startCols, unsigned int deltaRow, unsigned int deltaCol)
{
    PLAYER firstField = this->board[startRows][startCols];
    if(firstField == PLAYER::BLANK)
    {
        return false;
    } 
    for(int i = 0; i < CHECKS; i++)
    {
        int y = startCols + deltaCol * i;
        int x = startRows + deltaRow * i;
        if(this->board[y][x] != firstField)
        {
            return false;
        }
    }

    return true;
}

void Board::PrintBoard()
{
    for(int i = 0; i < ROWS; i++)
    {
        for(int j = 0; j < COLS; j++)
        {
            std::cout << " " << static_cast<char>(this->board[i][j]) << PLAYER::BLANK;
            std::cout << " | ";
            if(j == 2)
                std::cout << "\n";
        }
        if(i < 2)
            std::cout << "--------------\n";
    }
}

bool Board::InsertMove(BoardMove newMove)
{
    if(this->board[newMove.row][newMove.col] == PLAYER::BLANK)
    {

        this->board[newMove.row][newMove.col] = newMove.playerTurn;
        this->numberOfPlays++;
        this->CheckStatus(newMove.playerTurn);
        return true;

    }
    return false;
    
}

void Board::ClearBoard()
{

    for(int i = 0; i < ROWS; i++)
    {
        for(int j = 0; j < COLS; j++)
        {
            this->board[i][j] = PLAYER::BLANK;

        }
    }
    this->boardState = BOARDSTATUS::CLEAR;
}

I've made PLAYER and BoardMove publicly available because I call them directly from the my Game and AI classes. Should I make sure that they are only accessible from the Board Class and have functions to get that info?

Answer 1

Naming

In C++, as in C, we generally use ALL_CAPS names for preprocessor macros (as you have done) and not for other identifiers. This helps draw attention to the macros (which are textual substitutions, and therefore behave very different to language-level names).

Macros

We don't need to use macros for constant numbers. We can use static constexpr instead. Once away from the preprocessor, these values can be given more appropriate scope - I would suggest that they become static members of the Board class.

Visibility

board is public in the Board class. I think it would be better if it were private, so it can't be changed except through Board's functions.

Const

Board::PrintBoard shouldn't be modifying the contents, so declare that const (i.e. void PrintBoard() const). Instead of this function, we normally overload << like this:

friend std::ostream& operator<<(std::ostream&, Board const&);

That allows us to write to any output stream using the usual operator, rather than needing to know a function that can only write to std::cout.

Reduce redundancy

The Board destructor does nothing, so we don't need to declare or define it - let the compiler generate a default implementation for us.

In member functions, we automatically have access to the class's names - there's no need to write this-> everywhere.

In Board::CheckStatus(), win is a bool, so if (win == true) can be written simply if (win).

Data structures

Instead of an array of arrays, the code would be simpler with a singe rows ✕ cols array containing all the cells. That allows re-use of the same code to check rows, columns and diagonals just by changing how far we advance between each position (1 for horizontal, cols for vertical, cols+1 for leading diagonal and cols-1 for trailing diagonal). It also enables simpler initialisation (e.g. we can just use std::fill algorithm).

To help with adapting the existing code, we could use a simple helper function, so we can write at(x,y) anywhere we currently have board[x][y]:

private:
    Player& at(int x, int y)
    {
        return board[x * rows + y];
    }