Noughts and Crosses (Tic Tac Toe) C++ Console Application

Question

I was hoping someone could review my C++ console based noughts and crosses (Tic Tac Toe) application. I have not looked at any other documentation, this is purely programmed from the top of my head. Can you tell me what I've done well, what I haven't done well, and suggest any improvements I can make to it. If you decide to run this program ensure C++ 17 is specified (not that you need to be told that). Please treat it how it's supposed to be treated. I haven't included validation for inputs.

#include <iostream>
#include <vector>
#include <map>
#include <utility>
#include <algorithm>
void Displaygrid(const std::vector<std::vector<std::pair<int, unsigned char>>>&grid)
{
    std::for_each(grid.begin(), grid.end(), [&](auto pairVec) 
        {
        std::for_each(pairVec.begin(), pairVec.end(), [&](auto pair) 
            {
                if (pair.second != '-') {
                    std::cout << pair.second << "    ";
                }
                else 
                {
                    std::cout << pair.first << "    ";
                }
            }
        );
        std::cout << "\n";
       });
}

bool CheckForAWinner(const std::vector<std::vector<std::pair<int, unsigned char>>>&grid, const unsigned char key) 
{
    if (grid.at(0).at(0).second == key  && grid.at(0).at(1).second == key && grid.at(0).at(2).second == key) {
        std::cout << key << " is the winner" << std::endl;
        return true;
    }
    else if (grid.at(1).at(0).second == key && grid.at(1).at(1).second == key && grid.at(1).at(2).second == key) {
        std::cout << key << " is the winner" << std::endl;
        return true;
    }
    else if (grid.at(2).at(0).second == key && grid.at(2).at(1).second == key && grid.at(2).at(2).second == key) {
        std::cout << key << " is the winner" << std::endl;
        return true;
    }
    else if (grid.at(0).at(0).second == key && grid.at(1).at(0).second == key && grid.at(2).at(0).second == key) {
        std::cout << key << " is the winner" << std::endl;
        return true;
    }
    else if (grid.at(0).at(1).second == key && grid.at(1).at(1).second == key && grid.at(2).at(1).second == key) {
        std::cout << key << " is the winner" << std::endl;
        return true;
    }
    else if (grid.at(0).at(2).second == key && grid.at(1).at(2).second == key && grid.at(2).at(2).second == key) {
        std::cout << key << " is the winner" << std::endl;
        return true;
    }
    else if (grid.at(0).at(0).second == key && grid.at(1).at(1).second == key && grid.at(2).at(2).second == key) {
        std::cout << key << " is the winner" << std::endl;
        return true;
    }
    else if (grid.at(2).at(0).second == key && grid.at(1).at(1).second == key && grid.at(0).at(2).second == key) {
        std::cout << key << " is the winner" << std::endl;
        return true;
    }
    else if (grid.at(2).at(0).second == key && grid.at(1).at(1).second == key && grid.at(0).at(2).second == key) {
        std::cout << key << " is the winner" << std::endl;
        return true;
    }
    else 
    {
        return false;
    }
}
bool ClaimSquare(std::vector<std::vector<std::pair<int, unsigned char>>>& grid, int move, const unsigned char player) 
{
    for (auto& vec : grid) 
    {
        auto validSquare = std::find_if(vec.begin(), vec.end(), [&](auto pair) {
            return pair.first == move;
            });
        if (validSquare != vec.end()) 
        {
            if (validSquare->second == '-') 
            {
                validSquare->second = player;
                return true;
            }
            else
            {
                std::cout << "This square has already been claimed. Choose a different square!" << std::endl;
                return false;
            }
        }
    }
}
void PlayerMove(std::vector<std::vector<std::pair<int, unsigned char>>>& grid, unsigned char player) 
{
    int move = 0;
    do 
    {
        move = 0;
        Displaygrid(grid);
        std::cout << player << " turn: " << std::endl;
        std::cout << "Enter a number on the grid (e.g. 1): ";
        std::cin >> move;
    } 
    while (ClaimSquare(grid, move, player) == false);
}

void PlayNoughtsAndCrosses() 
{
    const unsigned char player1 = 'O';
    const unsigned char player2 = 'X';
    std::vector<std::vector<std::pair<int, unsigned char>>>grid =
    {
        {std::make_pair(1,'-'), std::make_pair(2,'-'), std::make_pair(3,'-') },
        {std::make_pair(4,'-'), std::make_pair(5,'-'), std::make_pair(6,'-') },
        {std::make_pair(7,'-'), std::make_pair(8,'-'), std::make_pair(9,'-') }
    };
    do 
    {
        PlayerMove(grid, player1);

        if (CheckForAWinner(grid, player1)) 
        {
            break;
        }

        PlayerMove(grid, player2);
    } 

    while (CheckForAWinner(grid, player2) == false);
}

int main()
{
    PlayNoughtsAndCrosses();
}

Answer 1

Overview

Not much in the way of OO design.
Not required but unusual for C++ application. With a bit of abstraction you can make multiple types of player all able to play the game (thus you could have a normal user or a computer plater play the game (see blow)).

Design

Your grid seems a bit over complex:

std::vector<std::vector<std::pair<int, unsigned char>>>grid =
{
    {std::make_pair(1,'-'), std::make_pair(2,'-'), std::make_pair(3,'-') },
    {std::make_pair(4,'-'), std::make_pair(5,'-'), std::make_pair(6,'-') },
    {std::make_pair(7,'-'), std::make_pair(8,'-'), std::make_pair(9,'-') }
};

Each time you need to find a square you search the grid. I mean its not that bad as the grid is very small. But you could have made that very easy by simply having an array of 9 squares and indexing directly into it.

char grid[9] = {'-', ....};

It would have made printing different but getting the square would be a lot simpler.

grid[move - 1] returns the square you are looking for.

Even if you had gone for a simple 2D array:

char grid[3][3] = {{'-', ....};

Then access to the square would have been:

grid[(move - 1) / 3][(move - 1) % 3] returns the square you are looking for.

Code Review

Looks like you could have designed the game loop in a simpler way:

const unsigned char player1 = 'O';
const unsigned char player2 = 'X';
do 
{
    PlayerMove(grid, player1);

    if (CheckForAWinner(grid, player1)) 
    {
        break;
    }

    PlayerMove(grid, player2);
} 
while (CheckForAWinner(grid, player2) == false);

Why not use a loop that switch between players?

int currentPlayer = 1;
const unsigned char player[2] = {'O', 'X'};
do 
{
    currentPlayer = (currentPlayer + 1) % 2;
    PlayerMove(grid, player[currentPlayer]);
} 
while (CheckForAWinner(grid, player[currentPlayer]) == false);

---

You have the same line twice:

else if (grid.at(2).at(0).second == key && grid.at(1).at(1).second == key && grid.at(0).at(2).second == key) {
    std::cout << key << " is the winner" << std::endl;
    return true;
}
else if (grid.at(2).at(0).second == key && grid.at(1).at(1).second == key && grid.at(0).at(2).second == key) {
    std::cout << key << " is the winner" << std::endl;
    return true;
}

---

Not sure the std::for_each() is best here. Why not use a range based for?

void Displaygrid(const std::vector<std::vector<std::pair<int, unsigned char>>>&grid)
{
    std::for_each(grid.begin(), grid.end(), [&](auto pairVec) 
        {
        std::for_each(pairVec.begin(), pairVec.end(), [&](auto pair) 

Like this:

void Displaygrid(const std::vector<std::vector<std::pair<int, unsigned char>>>&grid)
{
    for(auto const& row: grid) {
        for(auto const& cell: row) { 

OO Design

class Grid
{
    // Interface for board here:
};
class Player
{
    public:
        virtual ~Player() {}
        virtual int getNextMove(Grid const& grid) = 0;
};

class Game
{
    public:
        Game(Grid& board, Player& p1, Player& p2);
        Player& play(); // Play the game return a ref to winner.
};

class Human: public Player
{
    public:
        virtual int getNextMove(Grid const& grid)
        {
             // Ask user for their move (like your app above).
        }
};

class Robot: public Player
{
    public:
        virtual int getNextMove(Grid const& grid)
        {
             // Your AI code here.
        }
};