AL N*N Tic Tac Toe Game

Here is a simple Tic Tac Toe game. I would like to know how I can improve this code further.

```
#include
#include
#include
#include

enum struct Player : char
{
none = '-',
first = 'X',
second = 'O'
};

std::ostream& operator::type(p);
}

enum struct Type : int
{
row,
column,
diagonal
};

enum struct Diagonals : int
{
leftTopRightBottom,
rightTopleftBottom
};

template
class TicTacToe
{
public:
TicTacToe();

bool isFull() const;
void draw() const;
bool isWinner(Player player) const;
bool applyMove(Player player, std::size_t row, std::size_t column);

private:
std::size_t mRemain = DIM * DIM;
std::array mGrid;
};

template
struct Match
{
Match(Type t, int i)
: mCategory(t)
, mNumber(i)
{}

bool operator() (int number) const
{
switch (mCategory)
{
case Type::row:
return (std::abs(number / DIM) == mNumber);

case Type::column:
return (number % DIM == mNumber);

case Type::diagonal:
if (mNumber == static_cast(Diagonals::leftTopRightBottom))
{
return ((std::abs(number / DIM) - number % DIM) == mNumber);
}

if (mNumber == static_cast(Diagonals::rightTopleftBottom))
{
return ((std::abs(number / DIM) + number % DIM) == DIM - mNumber);
}

default:
return false;
}
}

Type mCategory;
int mNumber;
};

template
TicTacToe::TicTacToe()
{
mGrid.fill(Player::none);
}

template
bool TicTacToe::applyMove(Player player, std::size_t row, std::size_t column)
{
std::size_t position = row + DIM * column;

if ((position > mGrid.size()) || (mGrid[position] != Player::none))
{
return true;
}

--mRemain;

mGrid[position] = player;

return false;
}

template
bool TicTacToe::isFull() const
{
return (mRemain == 0);
}

template
bool TicTacToe::i

Several things worth pointing out here I think.

Match

There is zero reason for Match to take a category or really anything else. You don't need the Type or Diagonals enums. Given a move, you sohuld be able to determine if that move completed a row, column, or major diagonal.

// return true if given player won with the given move,
// false otherwise
bool won(Player, size_t row, size_t column);

TicTacToe

You may find it easier to just use a 2-d array to represent a 2-d object. That way:

bool applyMove(Player player, size_t row, size_t col)
{
    Player& square = mGrid.at(row).at(col);
    if (square == Player::none) {
        square = player;
        return true;
    }
    else {
        return false;
    }
}

Also you'll note I flipped the return type of applyMove. Returning true on success makes way more sense. Also this will throw std::out_of_range instead of silently doing nothing - that way you can catch that in Game and log an appropriate message. This differentiates an invalid move from attempting to play on a bad square.

Game

It is confusing to have getRandom be a member variable rather than a member function. I would change it. It took me a while to find the first time around. Now, turn() should just defer to the appropriate user:

void turn() {
    if (mPlayers[mPlayer] == Player::first) {
        userTurn();
    }
    else {
        compTurn();
    }
}

The pending loop is weird. Just have each sub-case return when they're done.

Also for the compTurn part, picking a random square each time is hugely inefficient. If there's only 1 open square left, you're spending on average N^2 loops to try to find it. Instead, first find the empty squares then pick a random one:

std::vector> open;
for (size_t r = 0; r < mDim; ++r) {
    for (size_t c = 0; c < mDim; ++c) {
        if (mGame.isOpen(r, c)) {
            open.emplace_back(r, c);
        }
    }
}

auto choice = chooseRandom(open);
applyMove(Player::second, choice.first, choice.second);

Also, the TicTacToe member object should really be named board. It's not the Game after all. The Game is already the Game.

Lastly, why is TicTacToe a class template? If you made the dimension a variable and used a vector> instead, you could ask for the dimension as input up front.

StackExchange Code Review Q#96484, answer score: 3