Tic Tac Toe in C++

Question

Here is my two-player Tic Tac Toe game. How can I improve it?

#include <iostream>
#include <vector>
//Simple Display
void Display(std::vector<char> const &grid){
    //Creating a onscreen grid
    std::cout << "   " << 1 << "  " << 2 << "  " << 3 << "\n";
    for(int a = 0; a < 9; a++){
        if(a == 0)
            std::cout << "A ";
        if(a == 3)
            std::cout << "\nB ";
        if(a == 6)
            std::cout << "\nC ";
        //displaying grid.
        std::cout <<  " " << grid[a] << " ";


    }
    std::cout << "\n\n";
}
//Returns true if the grid is already used.
bool Used(int const& position, std::vector<char> const& grid){
    if(grid[position] == '-')
        return false;
    else
        return true;
}

void Turn(std::vector<char> &grid, char player){
    int row = 0;
    char column = 0;
    int position = 0;
    bool check = true;
    std::cout <<"\n" << player << ": Please play. \n";

    while(check == true){

        std::cout << "Row(1,2,3): ";
        std::cin >> row;
        std::cout << player << ": Column(A,B,C): ";
        std::cin >> column;
        position = 3*(column-'A')+(row-1);
        if(!Used(position,grid)){
            check = false;
        }
        else{
            std::cout << "Already Used. Try Again. \n";
        }
    }
    grid[position] = player;
    std::cout << "\n\n";
}

bool Win(std::vector<char> const& grid, char player){
    for(int a = 0; a < 3; a++){
        if(grid[a] == player && grid[a+3] == player && grid[a+6] == player){
            return true;
        }
        if(grid[3*a] == player && grid[3*a+1] == player && grid[3*a+2] == player){
            return true;
        }
    }
    if(grid[0] == player && grid[4] == player && grid[8] == player){
        return true;
    }
    if(grid[2] == player && grid[4] == player && grid[6] == player){
        return true;
    }
    return false;

}

int main(){
    std::vector<char>grid(9,'-');

    while(true){
        Display(grid);
        Turn(grid, 'X');
        if(Win(grid, 'X')){
            Display(grid);
            std::cout << "\nX is the Winner!";
            break;
        }
        Display(grid);
        Turn(grid,'O');
        if(Win(grid, 'O')){
            Display(grid);
            std::cout << "\nO is the Winner!";
            break;
        }
    }
}

Answer 1

I see a number of things that would improve the code:

Use objects

C++ is an object-oriented language, and your code would benefit from being more object-oriented. When you have multiple functions that all take a grid as an argument, this is a clear indication that the functions should probably be member functions and that grid should be an object. Here's the class I came up with, based on your code. Note that the constructor takes an argument which defaults to 3. That's the size of one dimension of the square board, which can now be altered to be either the usual 3x3 grid (by default) or made any arbitrary size. More on that later.

class TicTacToe 
{
private:
    unsigned _dim;
    unsigned free;
    std::vector<char>grid;
    // applies turn and returns true unless square is already occupied
    bool Apply(char player, int position);

public:
    TicTacToe(unsigned dim=3) : _dim(dim), free(_dim*_dim), grid(free,'-') {}
    bool Full() const { return free==0; }
    //Simple Display
    void Display() const;
    void Turn(char player);
    bool Win(char player) const;
};

Separate input and output from core logic

Your Turn function does three things: it gets input from the user, it validates the input and then it applies the turn to the grid. It's generally better design to separate things so that each function does just one or maybe two closely related things.

When I changed your code, the Turn code is only responsible for getting the input, delegating the actual application of the move to a private member function named Apply:

bool TicTacToe::Apply(char player, int position) 
{
    if (grid[position] != '-')
        return false;
    grid[position] = player;
    --free;
    return true;
}

Consider extensions and generalizations

While your Win function is not incorrect as written, it is not flexible because all of the numbers are hard-coded. Consider that one might want a 4x4 or larger grid, in which it might be good to have a more general method for checking that doesn't have everything hard-coded.

bool TicTacToe::Win(char player) const 
{
    // check for row or column wins
    for(unsigned i = 0; i < _dim; ++i){
        bool rowwin = true;
        bool colwin = true;
        for (unsigned j=0; j < _dim; ++j) {
            rowwin &= grid[i*_dim+j] == player;
            colwin &= grid[j*_dim+i] == player;
        }
        if (colwin || rowwin) 
            return true;
    }
    // check for diagonal wins
    bool diagwin = true;
    for (unsigned i=0; i < _dim; ++i) 
        diagwin &= grid[i*_dim+i] == player;
    if (diagwin) 
        return true;
    diagwin = true;
    for (unsigned i=0; i < _dim; ++i) 
        diagwin &= grid[i*_dim+(_dim-i-1)] == player;
    return diagwin; 
}

The same thing can also be applied to your Display routine.

void TicTacToe::Display() const 
{
    //Creating a onscreen grid
    std::cout << ' ';
    for (unsigned i=1; i<=_dim; ++i)
        std::cout << "  " << i;
    for(unsigned i = 0; i < _dim; i++){
        std::cout << "\n" << static_cast<char>('A'+i) << " ";
        for(unsigned j = 0; j < _dim; j++)
            std::cout <<  " " << grid[i*_dim+j] << " ";
    }
    std::cout << "\n\n";
}

Recognize a draw

The code doesn't recognize when the game ends in a draw. An easy way to do so is to simply count how many free spaces are left. Note that this is only legitimately a tie if the last move hasn't actually been a winning move, so as written, this should only be called after we check for a win. This could be further improved by not relying on the calling function to perform this check.

bool Full() const { return free==0; }

Sanitize user input

User input is inherhently suspect. Make sure that you always validate user input before using it. In this case, consider that if the user enters a number instead of a letter, the code gets stuck in an infinite loop. Worse, the input is not range-checked and could attempt to write to memory far outside the grid vector.

void TicTacToe::Turn(char player)
{
    char row = 0;
    char column = 0;
    unsigned position = 0;
    bool applied = false;
    std::cout <<"\n" << player << ": Please play. \n";

    while(!applied) {
        std::cout << "Row(1,2,3,...): ";
        std::cin >> row;
        std::cout << player << ": Column(A,B,C,...): ";
        std::cin >> column;
        position = _dim*(column-'A')+(row-'1');
        if (position < grid.size()) {
            applied = Apply(player, position);
            if (!applied) 
                std::cout << "Already Used. Try Again. \n";
        } else {
            std::cout << "Invalid position.  Try again.\n";
        }
    }
    std::cout << "\n\n";
}

Highlight loop termination conditions

When you have a loop such as while(true) it implies that the loop never ends, but in the case of your loop in main, the code actually ends when one player wins. In the improved, code, it also ends if there is a tie.

int main()
{
    TicTacToe ttt;

    const char players[2] = {'X', 'O'};
    int player = 1;
    bool win = false;
    bool full = false;

    while(!win && !full){
        player = 1-player;
        ttt.Display();
        ttt.Turn(players[player]);
        win = ttt.Win(players[player]);
        full = ttt.Full();
    }
    ttt.Display();
    if (win) {
        std::cout << "\n" << players[player] << " is the Winner!\n";
    } else {
        std::cout << "\nTie game!\n";
    }
}

Answer 2

I'll just review what you have here, although something like this would work better with classes (you could always attempt this implementation later).

• Your vector isn't quite grid-like as it's only one-dimensional. It may also be why Display() is a bit confusing, which shouldn't be the case with a 2D structure.

  Either way, you just need an std::array since the board's size is fixed:

  std::array<std::array<char, 3>, 3> grid;
  

  You first need to include <array>, and you can then remove <vector>.

• In bool Used(), this:

  if(grid[position] == '-')
      return false;
  else
      return true;
  

  can simply become this:

  return !(grid[position] == '-');
  

  The statement already gives a conditional result, so you just have to return it.

  Also, you don't need to pass position by const& as it's a native type. Just pass by value, with const if you want to be safe.

• Don't just list variables at the start of Turn() (or in general). Declare or initialize them as close to their use as possible.

  For instance, here's what it should be for the user input portion:

  std::cout << "Row(1,2,3): ";
  int row;
  std::cin >> row;
  std::cout << player << ": Column(A,B,C): ";
  char column = 0;
  std::cin >> column;
  

  Side-note: row and column don't need be initialized; they can just be declared.

  Consider having input validation in case the user inputs a valid board spot, otherwise the program will stop working and the game will have to be restarted.

  You could also simplify the function and the input validation by using either letters or numbers for both row and column. Using different ones makes the code and interface more confusing.

• This:

  while(check == true)
  

  is the same as this but more concise:

  while(check)
  

  For false, use !:

  while(!check)
  

• The while loop in main() is quite repetitive. You could just have a bool assigned to the winning player, then display that value in the winning (or losing) message after the loop.

  You should also have a way of choosing whether to start with X or O. Normally, someone will choose to go first, and they will choose their own symbol.

Answer 3

One improvement you can make is to only use one set of routines for each player. One simple way to do this is with a char array to represent each player:

int main()
{
    std::vector<char>grid(9,'-');
    char players[] = {'X','O'};
    int player = 1;
    while(true)
    {
        player = abs(player - 1);
        Display(grid);
        Turn(grid, players[player]);
        if(Win(grid, players[player]))
        {
            Display(grid);
            std::cout << "\n" << players[player] << " is the Winner!";
            break;
        }
    }
}

One other thing I noticed, is the logic for the turns isn't accurate. The columns and rows seem to get swapped.