TicTacToe with MiniMax algorithm

I just got into C++ a few days ago and decided to make a MiniMax based TicTacToe game, I don't really know much about the conventions and best uses of the C++ language so I'd really appreciate any kind of feedback.

Brief summary:

• Board: has all the information pertinent to the game. Is used by the main method to paint the window and by the Brain to decide which move to make.

• Brain: Container for the MiniMax algorithm with helper methods. The Board is passed as a pointer to the Brain during construction (I don't know how wise this is but it's how I did things in Java).

• Main: mainly contains functions to draw the board and get inputs from the user.

Spacebar clears the board, F1 asks for a move, click performs a move.

Board.h - Most of the comments are not in the header files

#ifndef BOARD_H_
#define BOARD_H_
#include 

typedef enum {empty, x, o} cellState; // The 3 possible states a cell can be in

class Board {
public:
    Board();

    static const unsigned NUMBER_OF_CELLS = 9;
    static const unsigned CELLS_PER_ROW = 3;

    Board getClone();
    cellState getCell(int n);
    std::vector getLegalMoves();

    int getWinner();
    bool isGameOver();
    bool isXTurn();

    void applyMove(unsigned m);
    void clear();

    void print();

private:

    std::vector cells;
    cellState winner;
    bool xToMove;
    bool gameOver;
    unsigned movesMade;

    void setMark(int p, cellState m);
    void update();
    void xWon();
    void oWon();
    void draw();
};

#endif

Board.cpp

```
#include
#include "Board.h"

Board::Board() {
clear();
}

void Board::clear() { // Resets the game
cells.clear();
for (unsigned i = 0; i Board::getLegalMoves() { // Gets a vector of all the empty cells
std::vector moves;

for (unsigned i = 0; i = NUMBER_OF_CELLS || isGameOver())
return;

if (xToMove)
setMark(p, x);
else setMark(p, o);
}

void Board::print() { // Prints the board to the console for bugtesting purposes

Don't use raw pointers, make ownership semantics clear

class Brain {

public:
    Brain(Board* b);

    int getBestMove();
    void setBoard(Board* b); // Method not yet implemented, not needed for this small project

private:
    Board* board;
    // ...
};

Your Brain class uses a raw pointer to reference a current Board it is supposed to work with. Raw pointers leave us unclear about their ownership and who's responsible for the memory management.

You should rather use a smart pointer like std::unique_ptr (indicates ownership is transferred to the Brain class), std::shared_ptr (indicates ownership is shared) or std::weak_ptr (indicates Brain doesn't own that reference). I'll give a sample using std::shared_ptr:

class Brain {

public:
    Brain(std::shared_ptr b);

    int getBestMove();
    void setBoard(std::shared_ptr b);

private:
    std::shared_ptr board;
    // ...
};

Don't pass complex objects by value

int min(std::vector v);
int max(std::vector v);

You should use const references for these parameters:

int min(const std::vector& v);
int max(const std::vector& v);

Modern compilers may elide copies for these parameters, though semantics will be more clear and prevent you from accidentally changing those parameters inside the function.

Use const correctness

Since the min() or max() functions never change the state of the Brain class, these should be declared as const member functions:

int min(std::vector v) const;
int max(std::vector v) const;

In fact those functions don't even rely on any state stored in Brain, thus these shouldn't be class members at all but free functions. Best choice would be to use the already existing std::min_element() and std::max_element() functions from the standard library instead.

Prefer operator[] over at() to access container elements by index

The std::vector::at() function introduces bounds checking and will throw an exception if the requested index is out of bounds.

That's nice for debugging purposes with unchecked size beforehand.

For performance reasons you should prefer to do size checks beforehand and use the operator[] overload of the container class at all.

StackExchange Code Review Q#160870, answer score: 4