4
\$\begingroup\$

I am just beginning to learn coding and decided to create a text based tic tac toe board as my first solo C++ project. My goals were as follows:

  • be able to test if there is a winning state on any given board
  • be able to call the board state
  • have the computer play O's for any possible position
  • cleanly close out the game and announce the winner

Code is tested and bug free!

Some feedback on what I could improve on,what I did poorly, as well as any other general thoughts would be much appreciated!

#include <iostream>
#include <string>

// initialize some variables, give them each a different name so the player can choose a space
std::string a1{ "1" };
std::string a2{ "2" };
std::string a3{ "3" };
std::string b1{ "4" };
std::string b2{ "5" };
std::string b3{ "6" };
std::string c1{ "7" };
std::string c2{ "8" };
std::string c3{ "9" };




void boardState()
{
    //print the state of the board

    std::cout << "     |" << "     |" << '\n';
    std::cout << "  " << a1 << "  |" << "  " << b1 << "  |" << "  " << c1 << '\n';
    std::cout << "__________________" << '\n';
    std::cout << "     |" << "     |" << '\n';
    std::cout << "  " << a2 << "  |" << "  " << b2 << "  |" << "  " << c2 << '\n';
    std::cout << "__________________" << '\n';
    std::cout << "     |" << "     |" << '\n';
    std::cout << "  " << a3 << "  |" << "  " << b3 << "  |" << "  " << c3 << '\n';
}

void getMove()
{
    bool over{ false };
    
    while(over==false)
    {
        
        // ask user for their move
        std::cout << "Type your move(1-9): ";
        std::string temp{};
        std::cin >> temp;
        // take their move and if it is valid, place it on the board and exit
        if (temp == "1" && a1 == "1")
        {
            a1 = "X";
            over = true;
        }else if (temp == "2" && a2 == "2"){

            a2 = "X";
            over = true;
        }else if (temp == "3" && a3 == "3"){

            a3 = "X";
            over = true;
        }else if (temp == "4" && b1 == "4"){

            b1 = "X";
            over = true;
        }else if (temp == "5" && b2 == "5"){

            b2 = "X";
            over = true;
        }else if (temp == "6" && b3 == "6"){

            b3 = "X";
            over = true;
        }else if (temp == "7" && c1 == "7"){

            c1 = "X";
            over = true;
        }else if (temp == "8" && c2 == "8"){

            c2 = "X";
            over = true;
        }else if (temp == "9" && c3 == "9"){

            c3 = "X";
            over = true;
        }
        
    }
}

bool winCheck()
{
    // there are 8 total combinations of 3 placements that constitiute a win. manually check if any of these 8 occur and if one does return true
    
    if (a1 == a3 && a1 == a2)
    {
        std::cout << a1 << "'s WIN!";
        return true;
   
    }else if (a1 == b1 && a1 == c1){
        std::cout << a1 << "'s WIN!";
        return true;
   
    }else if (a1 == b1 && a1 == c1){
        std::cout << a1 << "'s WIN!";
        return true;
    
    }else if (a1 == b2 && a1 == c3){
        std::cout << a1 << "'s WIN!";
        return true;
    }else if (b1 == b2 && b1 == b3){
        std::cout << b1 << "'s WIN!";
        return true;
    }else if (c1 == b2 && c1 == a3){
        std::cout << c1 << "'s WIN!";
        return true;
    }else if (c1 == c2 && c1 == c3){
        std::cout << c1 << "'s WIN!";
        return true;
    }else if (a2 == b2 && a2 == c2){
        std::cout << a2 << "'s WIN!";
        return true;
    }else if (a3 == b3 && a3 == c3){
        std::cout << a3 << "'s WIN!";
        return true;
    }else{
        return false;
    }
}


void Oresponse()
{
    // the loop will check each space in this order; 5, 1, 9, 7, 3, 4, 6, 8, 2. and will place an O in the first available spot
    bool temp{ false };
    while (temp == false)
    {
        if (b2 == "5")
        {
            b2 = "O";
            temp = true;
        }else if(a1 == "1"){
            a1 = "O";
            temp = true;
        }else if(c3 == "9"){
            c3 = "O";
            temp = true;
        }else if(c1 == "7"){
            c1 = "O";
            temp = true;
        }else if(a3 == "3"){
            a3 = "O";
            temp = true;
        }else if(b1 == "4"){
            b1 = "O";
            temp = true;
        }else if(b3 == "6"){
            b3 = "O";
            temp = true;
        }else if(c2 == "8"){
            c2 = "O";
            temp = true;
        }else if(a2 == "2"){
            a2 = "O";
            temp = true;
        }else{
            temp = true;
        }
    }

}




int main()
{
    // until the gamestate is considered finished, loop printing the boardstate and prompting the user to make a move
    bool end{ false };
    while (end == false)
    {
        bool finished = winCheck();
        if (finished == true)
        {
            end = true;
            // if all possible board spots are filled end the game
        }else if(a1 != "1" && a2 != "2" && a3 != "3" && b1 != "4" && b2 != "5" && b3 != "6" && c1 != "7" && c2 != "8" && c3 != "9"){
          
            end = true;
        }else{
            boardState();
            getMove();
            Oresponse();
        }
    }

    boardState();
    winCheck();

    std::cout << std::endl;
    return 0;
}
\$\endgroup\$

2 Answers 2

5
\$\begingroup\$

Avoid writing stringly-typed code

Avoid using strings for things that could be better represented with other types, like for example integers or enums. For example, for reading in a move, read an integer:

std::cout << "Enter your move (1-9): ";
int position;
std::cin >> position;

We'll see the benefits of that later. For the board, consider that each position can be in three states. Create an enum for this:

enum class Cell {
    EMPTY,
    O,
    X,
};

You will lose a little bit of convenience, but we can make up for that in other ways.

Avoid code duplication

Instead of creating 9 variables to represent each cell of the board, create one array:

std::array<Cell, 9> cells{};

The huge advantage is now that we can reduce a lot of the code duplication. For example, getMove() can now be rewritten like so:

void getMove()
{
    while (true)
    {
        std::cout << "Enter your move (1-9): ";
        int position;
        std::cin >> position;

        // Get the cell at the given position
        auto& cell = cells.at(position - 1);

        // take their move and if it is valid, place it on the board and exit
        if (cell == Cell::EMPTY)
        {
            cell = Cell::X;
            break;
        }        
    }
}

Also consider converting boardState() to use for-loops to print the board. That sounds a bit silly for a 3x3 board, but once you have done that with all the code, then it suddenly becomes trivial to, for example, change it to print a 5x5 board.

In Oresponse(), you write:

// the loop will check each space in this order; 5, 1, 9, 7, 3, 4, 6, 8, 2. and will place an O in the first available spot

And then you have a while-loop with a huge if-statement inside, which is equivalent to the loop-switch antipattern. Instead, create an array holding the sequence:

static const std::array<int, 9> positions{5, 1, 9, 7, 3, 4, 6, 8, 2};

Then similar to what I've shown above, you can simplify your loop to:

for (int position: positions) {
    auto& cell = cells[position - 1];
    if (cell == Cell::EMPTY) {
        cell = Cell::O;
        break;
    }
}

Missing error checking

Any I/O operation can fail at any point in time. Even reading from std::cin can fail: perhaps the user closed the terminal, or pressed control-D on UNIX, or the input was redirected from some file or pipe. Make sure you check that the input was actually read correctly, and if there is an unrecoverable error, exit your program with a non-zero status code.

After reading some input, use std::cin.fail() to check if it failed to parse the input, and std::cin.bad() to check if the stream itself is bad. In the former case, you can just ask the user to try to enter a move again, in the latter case there is no point anymore. So:

std::cin >> position;

if (std::cin.fail()) {
    continue;
} else if (std::cin.bad()) {
    std::exit(EXIT_FAILURE);
}

Consider creating a class Board

You already organized your code into separate functions, which is great. But consider creating a class Board to represent the board, and have member functions which query and modify the state of the board. Interaction with the user should not be in the member functions though, to keep the responsibilities of this class to a minimum.

For printing the board, you already have boardState(), but wouldn't it be nice if you could just write std::cout << board? You can do that by making it an overload of the operator<<(std::ostream&).

I will not elaborate further on this, but have a look at some of the other submissions with C++ tic-tac-toe implementations on this site that do use a class to store the state of the board. In particular:

  • Tic-Tac-Toe Console Game: very nice code, does have the std::ostream overload, but uses just a char to store a cell's state.
  • AL N*N Tic Tac Toe Game - 2: a bit more complicated as it works for any size board, but it shows you how you can avoid hardcoding the size.

Testing

Code is tested and bug free!

That's great! But how did you test it? If it was manual, then when you make a change to your code, you'd have to do all that manual testing again to make sure it still works as expected. Consider writing a test suite for your code.

If you have a class Board and a print function that is an overload of operator<<(std::ostream&) as mentioned above, then code that tests the functionality of Board becomes quite easy to write.

\$\endgroup\$
8
  • 1
    \$\begingroup\$ Im working through all your suggestings trying my best to understand what is happening and im having trouble getting this codeblock to work ``` for (int position: positions) { auto& cell = cells[positions - 1]; if (cell == Cell::EMPTY) { cell = Cell::O; break; } } ``` the error im getting is no operator "-" matches these operandsC/C++(349) main.cpp(112, 38): operand types are: const std::array<int, 9UL> - int \$\endgroup\$ Commented Jun 8, 2023 at 18:03
  • \$\begingroup\$ Sorry, I made a typo in my example, it should be cells[position - 1] (singular of position, not plural). \$\endgroup\$
    – G. Sliepen
    Commented Jun 8, 2023 at 18:23
  • \$\begingroup\$ I'm trying my best to implement the suggestions but im having a really hard time setting up the board class. how can I refrence a specific cell of the cells array within the board class in order to print out the state of the board? having cell 3 states in the enum is well and good but I havnt figured out how to print out the board using them. I keep running into the problem where it doesnt want to print out any of the 3 states because they arent strings. Thank you for responding to my questions you have been enormously helpful. \$\endgroup\$ Commented Jun 9, 2023 at 3:10
  • \$\begingroup\$ @AliceAaron-force: Using char values of 'X', 'O', and ' ' for empty, is totally fine. It lets you skip a step of mapping an enum value to a char when you want to print. Although it should be trivial to write a function like char mark_to_char(enum class Cell mark) with a switch or table lookup. Things don't have to be strings; single-byte char is good and compiles efficiently since it's also just an integer type, like int but narrower. (In some ways like enum.) \$\endgroup\$ Commented Jun 9, 2023 at 3:43
  • 1
    \$\begingroup\$ You can still define constants like static const char X = 'X'; inside a class if you want to use something like Cell::X instead of hard-coding your game logic to use a constant like 'X' that was influenced by UI choice (wanting to output as ASCII text.) \$\endgroup\$ Commented Jun 9, 2023 at 3:45
2
\$\begingroup\$

I won't repeat everything G. Sliepen pointed out, but use arrays is such an important point it bears repeating. C-style char board[3][3]; or std::array< std::array<char, 3>, 3> board; would work.

Or a 1D std::array<char, 9> board; maybe with some enum constants for indexing it like board[pos_c3] instead of board[0][2] or board[0*3 + 2]. But you don't want to hide the indexing completely, the point is to make it possible to loop over rows of the array. Obviously the elements should be char not std::string since you only ever want a single character. (You can use ' ' for empty if you want to allow an alternate UI where cells start empty instead of with numbers.)


UI design: you number the grid like this:

# original program
     |     |
  1  |  4  |  7
__________________
     |     |
  2  |  5  |  8
__________________
     |     |
  3  |  6  |  9

The number pad on normal keyboards is a 3x3 grid with a different layout. Matching it would make it much more natural to enter moves by pressing e.g. the bottom left key to mark the bottom-left space in the grid on the screen.

# PC/Mac keyboard number pad
     |     |
  7  |  8  |  9
__________________
     |     |
  4  |  5  |  6
__________________
     |     |
  1  |  2  |  3

Comments

Checking for empty spaces in a set order is very easy to implement, and maybe slightly more interesting to play against than if the order was just scanning along rows. The only comment in that function isn't very helpful, though:

// the loop will check each space in this order; 5, 1, 9, 7, 3, 4, 6, 8, 2. and will place an O in the first available spot

We can already see the order from looking at the chain of if() statements. What would be useful is to summarize what those squares are. I had to go through the numbers and look back and forth at a grid to see the pattern, since there wasn't a comment like:

// play in the first available spot of: centre, corners, non-corners

The art of writing comments is to add information that isn't already obvious from reading the code. Code should already be written so other humans can see what it does, the interesting part is why. (Sometimes the "what" part can get a little tricky, and comments there can help, but in this case the code is already structured like the list of positions.)


Computer move selection

This fixed move-selection is of course very easy to beat, failing to defend against a winning move on the very next turn. And will miss its own chances to win in one move. So it's not any fun to play against, except to see how the program works. (Not that Tic Tac Toe is much fun to play on its own, except with children.)

Tic Tac Toe is always a tie with best play from both players; neither one can force a win without a mistake. But the mistake might not be obvious right away: letting the opponent get to a position where they have two different winning moves means you can't block both. Tic Tac Toe is such a small game that CPUs are fast enough to run a simple exhaustive recursive search to explore all possible moves by either side until win or tie, from the current position. (You might want to special case the empty-board start of the game, instead of fully solving Tic Tac Toe by checking every possible game. About 25 years ago, this took a second or two on computers at the time in the TTT program I wrote in C for a homework assignment, using a struct board { char state[3][3]; } so I could copy it around by value to keep track of different game states. Actually I just checked my old code and I didn't wrap it in a struct, just a typedef, so I had to memcpy. It was one of the first interesting / memorable C programs I ever wrote. :P)

A perfect AI that never makes a losing move is a fun exercise, but the next step beyond a fixed move selection would be just to look one move ahead:

If there's a winning move anywhere, make it. If an X in a location would create a win for the human player, play there. If neither of those, then do random or a fixed sequence.

You already have a function that checks for wins. But it works with global variables, and worse, prints to cout instead of just returning a status, so you couldn't use it to check what would happen if you played somewhere. (Even worse, you repeat the std::cout << statement in every if block.) As a general rule, separate UI from game logic. One way to do this would be to return a char that's either 0 for no winner, or 'X' or 'O'.

That gives you 3 states instead of 2 from a bool return value. The calling function would then do char winner = winCheck(board); if (winner) { ... }, because zero vs. non-zero integers are implicitly false vs. true in boolean contexts in C and C++. (And char is an integer type.) You could also write it as if (winner != 0) if you like.

So my main point here is that writing functions like winCheck to not print themselves will let you keep using them without as much re-factoring when you do want to make your game logic more sophisticated. This is also related to the Single responsibility principle on a smaller scale.

Note that if X just moved, the only possible winner is X. If O had 3 in a row, you'd already have detected that after they moved. If you were using ' ' instead of numbers as a placeholder for empty squares, you might have the caller pass an arg of which side to check for, since looking for equality would detect whitespace as a "winner". But you don't have that problem since you use ASCII digits, so your winner function can just check board[0][0] == board[0][1], etc.

A simple way to look for winning moves is just to save the old state at an empty square, mark it X or O, and check for a winner. If not, then restore the state and move on. (Or make a copy of the whole board state and modify that.)

A more efficient way might look for board[0][0] == board[0][1] if board[0][2] is open, since either you win by playing there, or you block an opponent's win. But it does matter which: blocking one winning move might just lead to a draw or still a loss, but making a winning move definitely wins. So you might remember the first or last forced defensive move you find, while you keep searching for a possible winning move. But this would require more custom search code, not just a simple loop calling a non-printing version of winCheck.

\$\endgroup\$
1
  • \$\begingroup\$ Your indepth breakdown into the computer response will give me a lot of insight later down the road when I have the game itself built more solidly with G. sliepen's suggestings. thank you so much! \$\endgroup\$ Commented Jun 9, 2023 at 3:14

Not the answer you're looking for? Browse other questions tagged or ask your own question.