Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Logical Mistakes, Checking for a Winner in Tic Tac Toe

Alright, so I am incredibly close to finishing this program. I understand why my program wasn't making a move and I was able to fix it, but now I am trying to check for a winner. I realize that my winGame() function should be in some sort of while or do while loop to end the game. But, when I was trying to do a little debugging to sort some things out, I realize something unsettling. It always says that it's a draw, even when it's not supposed to be. These are such minor things that I am ashamed to not understand and I would really like some assistance on how I can fix it. Also, I know there's supposed to be a while or do while loop to end the game if there's a win. I'm just unsure where to put it, so if you have any suggestions, please let me know.

*Note that in my valid move function there's a little array, I plan on making it a static const array. My get functions return the value in the name (e.g. getIval() returns the initial value of the cell object), while my set functions just assign the values appropriately.

bool TicTacToe::validMove(char move){
    char options[9] = { '1','2', '3', '4','5','6','7', '8','9' };
    bool validate = false;
    for ( int i = 0; i < 9; i++ ){
        if ( move == options[i]){
            validate = true;
        }
    }

    return ( validate );
}

void TicTacToe::setMove( char move ){
    for ( int i = 0; i < ROW; i++ ){
        for ( int j = 0; j < COL; j++ ){
            if ( board[i][j].getiVal() == move ){
                board[i][j].setiVal( players[currentPlayer].getMarker() );
                switchPlayer();
                break;
            }
        }
    }
}

void TicTacToe::makeAMove(){
    char move;
    int turns = 1;
    bool validate = true;

    do{
        cout << "Player " << (getCurrentPlayer() + 1) << " make a move." << endl;
        cin >> move;

        if ( validMove( move ) ){
            if ( turns > 4 ){
                cout << "Nested if-else statement." << endl;
                winGame();
                setMove( move );
            }
            else
                setMove(move);
        }
        else{
            cout << "Invalid Move. Please reenter." << endl;
            cin >> move;
        }

        DrawBoard();
        turns++;

    } while ( turns <= 9 );   
}

bool TicTacToe::winGame(){
    cout << "Calling winGame() "  << endl;
    bool validate = false;
    int k = 0;
    for ( int i = 0; i < COL; i++ ){
        //check column wins
        if ( board[0][i].getMarker() == board[1][i].getMarker() && board[1][i].getMarker() == board[2][i].getMarker() && board[2][i].getMarker() != (' ')){
            cout << "Column win " << endl;
            validate = true;
            break;
        }
        //check row wins
        else if ( board[i][0].getMarker() == board[i][1].getMarker() && board[i][1].getMarker() == board[i][2].getMarker() && board[i][2].getMarker() !=  (' ')){
            cout << "Row win." << endl;
            validate = true;
            break;
        }
    }

    if( board[0][0].getMarker() == board[1][1].getMarker() && board[1][1].getMarker() == board[2][2].getMarker() && board[2][2].getMarker() != (' ')){
        cout << "Diagonal 1" << endl;
        validate = true;
    }
    else if ( board[0][2].getMarker() == board[1][1].getMarker() && board[1][1].getMarker() == board[2][0].getMarker() && board[2][0].getMarker() != (' ') ){
        cout << "Diagonal 2 " << endl;
        validate = true;
    }
    else{
        cout << "It's a draw!" << endl;
        validate = true;
    }

    return (validate);
}

Here is a sample run of the program for your reference.

    //sample run
+--+--+--+
|1 |2 |3 |
+--+--+--+
|4 |5 |6 |
+--+--+--+
|7 |8 |9 |
+--+--+--+
Player 1 make a move.
1

+--+--+--+
|X |2 |3 |
+--+--+--+
|4 |5 |6 |
+--+--+--+
|7 |8 |9 |
+--+--+--+
Player 2 make a move.
2

+--+--+--+
|X |O |3 |
+--+--+--+
|4 |5 |6 |
+--+--+--+
|7 |8 |9 |
+--+--+--+
Player 1 make a move.
3

+--+--+--+
|X |O |X |
+--+--+--+
|4 |5 |6 |
+--+--+--+
|7 |8 |9 |
+--+--+--+
Player 2 make a move.
5

+--+--+--+
|X |O |X |
+--+--+--+
|4 |O |6 |
+--+--+--+
|7 |8 |9 |
+--+--+--+
Player 1 make a move.
+--+--+--+
|X |O |X |
+--+--+--+
|4 |O |6 |
+--+--+--+
|7 |8 |9 |
+--+--+--+
Player 1 make a move.
7
Nested if-else statement.
Calling winGame()
It's a draw!

+--+--+--+
|X |O |X |
+--+--+--+
|4 |O |6 |
+--+--+--+
|X |8 |9 |
+--+--+--+
Player 2 make a move.
8
Nested if-else statement.
Calling winGame()
It's a draw!

+--+--+--+
|X |O |X |
+--+--+--+
|4 |O |6 |
+--+--+--+
|X |O |9 |
+--+--+--+
like image 884
Chynna Hernandez Avatar asked Sep 25 '22 21:09

Chynna Hernandez


2 Answers

It always says that it's a draw, even when it's not supposed to be.

The reason is that your winGame function doesn't return immediately on detection of a row or column winner. Instead, if a row or column wins, additional checks are done to check the diagonal winner for no reason.

The code should return immediately on detection of a column or row winner instead of running into the diagonal checks. There also would be no need for a validate variable if the code were done this way.

It would be better if you took a more systematic approach and just write the code for the 3 various ways to win: by row, by column, and by diagonal. If any of those are winners, then return immediately.

Also, it is faster to check if there is a marker first before checking row, column, or diagonals. Your code does the marker check for blank last, thus needlessly calling getMarker when there is no need to call it.

The code illustrates the points made:

bool TicTacToe::winGame()
{
    char marker;

    // row check
    for ( int i = 0; i < COL; i++ )
    {
       marker = board[i][0].getMarker();  // get the initial marker
       // test if something is there
       if ( marker != ' ')
       {
          // now test the other two markers to see if they match
          if (  board[i][1].getMarker() == marker && 
                board[i][2].getMarker() == marker )
            return true;
       }
    }

    // column check
    for ( int i = 0; i < COL; i++ )
    {
       marker = board[0][i].getMarker();
       if ( marker != ' ')
       {
          if (  board[1][i].getMarker() == marker && 
                board[2][i].getMarker() == marker )
            return true;
       }
    }

    // check diagonals next
    //... (code not shown)
    return false; // if the diagonals fail
}

I didn't write the code to test the diagonals, but you should get the idea. The row and column checks are done in separate loops (nothing fancy). If there is a winner within any iteration of those loops, the return value is true, denoting a winner.

like image 180
PaulMcKenzie Avatar answered Oct 03 '22 14:10

PaulMcKenzie


There are 3 problems with this code.

  1. The game loop does not end upon victory.
  2. The win function does not return as soon as victory is confirmed.
  3. The logic for the draw condition is incorrect.

these are easily fixed though.

  • in your do-while loop place a break if WinGame==true
  • change the breaks in row and column victories with return validate
  • Pass turns into the WinGameme function and make an extra if statement that checks if turns==9

    void TicTacToe::makeAMove(){ char move; int turns = 1; bool validate = true;

        do{
                cout << "Player " << (getCurrentPlayer() + 1) << " make a move." << endl;
                cin >> move;
    
                if ( validMove( move ) ){
                        if ( turns > 4 ){
                                cout << "Nested if-else statement." << endl;
    
                                setMove( move );
                                if (winGame(turns)==true)
                                {
                                    break;
                                }
                        }
                        else
                                setMove(move);
                }
                else{
                        cout << "Invalid Move. Please reenter." << endl;
                        cin >> move;
                }
    
                DrawBoard();
                turns++;
    
        } while ( turns <= 9 );
        cout << "Game Over" <<endl;
    

    }

and then

bool TicTacToe::winGame(int turns)
{
        cout << "Calling winGame() "  << endl;
        bool validate = false;
        int k = 0;
        for ( int i = 0; i < COL; i++ )
        {
                //check column wins
            if ( board[0][i].getMarker() == board[1][i].getMarker() &&
                 board[1][i].getMarker() == board[2][i].getMarker() &&
                 board[2][i].getMarker() != (' ')){
                        cout << "Column win " << endl;
                        validate = true;
                        break;
                }
                //check row wins
                 else if ( board[i][0].getMarker() == board[i][1].getMarker() &&
                           board[i][1].getMarker() == board[i][2].getMarker() && 
                           board[i][2].getMarker() !=  (' ')){
                        cout << "Row win." << endl;
                        validate = true;
                        break;
                }
        }

        if( board[0][0].getMarker() == board[1][1].getMarker() && 
            board[1][1].getMarker() == board[2][2].getMarker() && 
            board[2][2].getMarker() != (' ')){
                cout << "Diagonal 1" << endl;
                validate = true;
        }
        else if ( board[0][2].getMarker() == board[1][1].getMarker() &&
                  board[1][1].getMarker() == board[2][0].getMarker() &&
                  board[2][0].getMarker() != (' ') ){
                cout << "Diagonal 2 " << endl;
                validate = true;
        }
        else
        {
            if (turns==9)
                {
                    cout << "It's a draw!" << endl;
                    validate = true;
                }
        }

        return (validate);
} 
like image 25
Pomadomaphin Avatar answered Oct 03 '22 15:10

Pomadomaphin