-2

I recently started learning c++ and I'm trying to make a tic-tac-toe game.

I'm using a vector for the board and modifying the board once per player turn.

The board looks like this:

std::vector<char> board = { '-', '-', '-', '-', '-', '-', '-', '-', '-', '-', };

Here is the function modifying the board:

int player_turn(std::vector<char> board) {
    int guess;
    std::cout << "Please enter field 1-9: \n";
    std::cin >> guess;
    if (guess < 10 && guess > 0 && board[guess-1] == '-') return(guess);
    else {
        std::cout << "Invalid input! Try again\n";
        player_turn(board);

    }
}

This is how I'm calling the function in my main file:

board[player_turn(board) - 1] = 'O';

If I run the program and only enter valid guesses that don't trigger the recursion it works without issues. However if I try entering an invalid input that doesn't trigger the if block, as long as I enter invalid inputs the else block triggers and the function runs again. But if I enter an invalid input, and then a valid input I get the error Expression: vector subscript out of range. As far as I can tell that error comes up when I try to access an index in the vector that doesn't exist. What I don't understand is why that is happening. I can't figure out what modifies the vector after an invalid input when it shouldn't change at all until a valid input is there. Thanks in advance.

fabian
  • 80,457
  • 12
  • 86
  • 114

2 Answers2

2

You're experiencing undefined behavior since your function doesn't return a value in each branch.

int player_turn(std::vector<char> board) {
    int guess;
    std::cout << "Please enter field 1-9: \n";
    std::cin >> guess;
    if (guess < 10 && guess > 0 && board[guess-1] == '-') return(guess);
    else {
        std::cout << "Invalid input! Try again\n";
        return player_turn(board);
   //   ^^^^^^

    }
}

There are many ways you can avoid such mistakes.

First, you could enable warnings. Using GCC or Clang, add the -Werror=return-type compiler flag to make such code not compile.

Using MSVC, such code will not compile by default.

You can also mark your function as nodiscard:

[[nodiscard]]
int player_turn(std::vector<char> board) {
    // ...
}

This will make the compiler emit a warning if you call the function but ignore the result.

Guillaume Racicot
  • 39,621
  • 9
  • 77
  • 141
0

Recursion is a really bad idea here, but it's not the cause of the problem, so we'll roll with it for this question.

else {
    std::cout << "Invalid input! Try again\n";
    player_turn(board);  // <- HERE IS YOUR PROBLEM
}

In the else clause you forgot to add return.

In sufficiently old and noncompliant compilers you won't get a compiler error (so you should use a better one), but from C++11 on this is undefined behavior as per this question.

This will fix the problem:

else {
    std::cout << "Invalid input! Try again\n";
    return player_turn(board);  
}

Or better yet, remove the recursion:

int player_turn(std::vector<char> const &board) 
{
    int guess;
    do {
        std::cout << "Please enter field 1-9: \n";
        std::cin >> guess;
        if (guess < 10 && guess > 0 && board[guess-1] == '-') 
           return guess;

        std::cout << "Invalid input! Try again\n";
    } while (true);
}
Spencer
  • 1,924
  • 15
  • 27
  • In what sense is this a syntax error? – Nathan Pierson Oct 17 '22 at 16:59
  • @NathanPierson It's a syntax error by default in MSVC and you can set it to be an error in gcc. But also in the moral sense: There's never a good reason to invoke undefined behavior. – Spencer Oct 17 '22 at 17:04
  • I agree that it's an error, I disagree that it's a _syntax_ error specifically. There's no part of the syntax that's invalid, the problem is that the sum of the individually-valid lines is a function exhibiting undefined behavior. – Nathan Pierson Oct 17 '22 at 17:06
  • @NathanPierson MSVC always treats this as an error. – Spencer Oct 17 '22 at 17:06
  • Hey thanks for the help. I started c++ yesterday so I didn't know you need to return the recursion for it to work. I don't quite understand the way you removed recursion and how the do block even works. Does it just repeat until it returns a value or? And how does the while (true) effect it? Also I'm using visual studio to write c++ and installed it a couple days ago so I don't know how it could be a really old compiler as an issues. – Goran Orsolic Oct 17 '22 at 17:07
  • @GoranOrsolic `do { ... } while (true);` repeats forever, except that we're breaking out of the loop when we `return` something. – Spencer Oct 17 '22 at 17:08