AL N*N Tic Tac Toe Game - 2

Question

Base on the previous question:

AL N*N Tic Tac Toe Game

Here is a summary of improvements:

• Deleting the Match class for its expensive calling constructor
• Deleting enum struct Type and enum struct Diagonals for no longer being needed
• Converting Random class to a template to allow either int or unsigned values as parameters

How can I improve this game?

#include <iostream>
#include <cctype>
#include <array>
#include <random>

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

std::ostream& operator<<(std::ostream& os, Player const& p)
{
    return os << std::underlying_type<Player>::type(p);
}

// TicTacToe Class takes care for the logic and the drawing of the game.
template<std::size_t DIM> // main reason for template is avoiding global variables
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<Player, DIM * DIM> mGrid;
};

template<std::size_t DIM>
TicTacToe<DIM>::TicTacToe()
{
    mGrid.fill(Player::none);
}

template<std::size_t DIM>
bool TicTacToe<DIM>::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<std::size_t DIM>
bool TicTacToe<DIM>::isFull() const
{
    return (mRemain == 0);
}

template<std::size_t DIM>
bool TicTacToe<DIM>::isWinner(Player player) const
{
    std::array<bool, 2 * (DIM + 1)> win;

    win.fill(true);

    int j = 0;

    for (auto i : mGrid)
    {
        int x = j++;

        for (std::size_t k = 0; k < DIM; ++k)
        {
            if (x % DIM == k)
            {
                win[k] &= i == player;
            }

            if (x / DIM == k)
            {
                win[DIM + k] &= i == player;
            }

            if ((k == 0 && (x / DIM - x % DIM == k))            // Diagonals -> LeftTop RightBottom
            || (k == 1 && (x / DIM + x % DIM == DIM - k)))      // Diagonals -> RightTop leftBottom
            {
                win[2 * DIM + k] &= i == player;    
            }
        }
    }

    for (auto i : win)
    {
        if (i)
        {
            return true;
        }
    }

    return false;
}

template<std::size_t DIM>
void TicTacToe<DIM>::draw() const
{
    std::cout << ' ';
    for (std::size_t i = 1; i <= DIM; ++i)
    {
        std::cout << "  " << i;
    }

    int j = 0;
    char A = 'A';

    for (auto i : mGrid)
    {
        if (j == 0)
        {
            std::cout << "\n " << A++;
            j = DIM;
        }
        --j;

        std::cout << ' ' << i << ' ';
    }

    std::cout << "\n\n";
}

template<typename T>
class Random
{
public:
    Random(const T& min, const T& max)
        : mUnifomDistribution(min, max)
    {}

    T operator()()
    {
        return mUnifomDistribution(mEngine);
    }

private:
    std::default_random_engine mEngine{ std::random_device()() };

    template <typename U>
    static auto dist() -> typename std::enable_if<std::is_integral<U>::value, std::uniform_int_distribution<U>>::type;

    template <typename U>
    static auto dist() -> typename std::enable_if<std::is_floating_point<U>::value, std::uniform_real_distribution<U>>::type;

    decltype(dist<T>()) mUnifomDistribution;
};

// Game class represent the game loop for the tic tac toe game 
// it simply takes inputs by switching users to check for whom is the winner. 
class Game 
{
public:
    void run();

private:
    void showResult() const;
    void turn();

    static const std::size_t mDim = 4;
    static const std::size_t mNumberOfPlayers = 2;
    TicTacToe<mDim> mGame;
    std::array<Player, mNumberOfPlayers> mPlayers{ { Player::first, Player::second } };
    int mPlayer = 1;
    Random<int> getRandom{ 0, mDim - 1 };
};

void Game::run()
{
    while (!mGame.isWinner(mPlayers[mPlayer]) && !mGame.isFull())
    {
        mPlayer ^= 1;
        mGame.draw();
        turn();
    }

    showResult();
}

void Game::showResult() const
{
    mGame.draw();

    if (mGame.isWinner(mPlayers[mPlayer]))
    {
        std::cout << "\n" << mPlayers[mPlayer] << " is the Winner!\n";
    }
    else
    {
        std::cout << "\nTie game!\n";
    }
}

void Game::turn()
{
    char row = 0;
    char column = 0;

    for (bool pending = true; pending;)
    {
        switch (mPlayers[mPlayer])
        {
        case Player::first:
            std::cout << "\n" << mPlayers[mPlayer] << ": Please play. \n";
            std::cout << "Row(1,2,3,...): ";
            std::cin >> row;
            std::cout << mPlayers[mPlayer] << ": Column(a,b,c,...): ";
            std::cin >> column;

            column = std::toupper(column) - 'A';
            row -= '1';

            pending = column < 0 || row < 0 || mGame.applyMove(mPlayers[mPlayer], row, column);

            if (pending)
            {
                std::cout << "Invalid position.  Try again.\n";
            }
            break;
        case Player::second:
            row = getRandom();
            column = getRandom();

            pending = mGame.applyMove(mPlayers[mPlayer], row, column);
            break;
        }
    }

    std::cout << "\n\n";
}

int main()
{
    Game game;
    game.run();
}

Answer 1

This is a cool idea! I like that you can change the size of the board. Sounds like a fun game. Here are my suggestions.

Use the Right Tool for the Job

I'm not sure I understand the point of TicTacToe being a template and not a regular class. If it were just a class, you could pass the dimensions of the board into a constructor. Having a template where the only thing that varies is the size of some internal storage doesn't seem like the best use of templates to me.

Readability

The applyMove() method returns true if it fails and false if it succeeds. That's counterintuitive. I'd expect the opposite.

I had a hard time understanding your isWinner() method. It's a very strange way to test for the winning condition. I think a straightforward implementation where you manually iterate over each row and column would be easier to understand and maintain. At the very least, some comments in the one you have would be nice. (Also, are you sure you've allocated enough space in the win array? It looks to me like it needs to be 3 * DIM, not 2 * (DIM + 1).) It seems like you really need 3 separate arrays and you've just made them all a single array and you're using different sections of the array to represent rows, columns, and diagonals.

The Game

In the Game class, it looks like mPlayer could be a static const std::array since it never changes and is the same for every instance.

In Game::run(), this:

mPlayer ^= 1;

is too clever. It's the type of thing that's not obvious so it's best avoided. Just us the simpler:

mPlayer = (mPlayer + 1) % 2;

In Game::turn(), you have this tortured for loop:

for (bool pending = true; pending;)

It's better written as a while loop:

bool pending = true;
while (pending)
// ... etc.