The CPU displays more turns than human in the Tic-Tac-Toe board

[kabirdevramana96]

Hi there. I am having a problem with the number of turns the human and CPU displays on the board in the Tic-Tac-Toe game. The CPU displays more turns than the human. I am attaching my source code below for you guys to have a look.
#include "pch.h"
#include "TicTacToe.h"
#include<iostream>
#include<string>
using namespace std;

TicTacToe::TicTacToe()
{
cout << "Welcome to the Tic-Tac-Toe game" << endl;

//Intialzing values of attributes from class

size = 1;
n = 0;
matrix[0][0] = '1';
matrix[0][1] = '2';
matrix[0][2] = '3';
matrix[1][0] = '4';
matrix[1][1] = '5';
matrix[1][2] = '6';
matrix[2][0] = '7';
matrix[2][1] = '8';
matrix[2][2] = '9';
human_turn = 'X';
Computer_turn = 'O';


}



void TicTacToe::Run_game()
{
bool game_over = false;


while (!game_over) {
n++;
Draw();
human_turns();
CPU_turns();
if (Results() == human_turn) {
cout << "Player 1 wins" << endl;
break;
}
else if (Results() == Computer_turn) {
cout << "Player 2 wins" << endl;
break;
}
else if (Results() == '/' && n == 9) { // Comparing if result is a draw or not
cout << "The game ends in a draw" << endl;
cout << "You should play the game again to find a winner" << endl;
break;
}
Set_players();
}

}


void TicTacToe::Draw()
{
//Nested for loop to loop through the rows and columns of matrix
for (int i = 0; i < size; i++) {
for (int j = 0; j < size; j++) {

cout << " " << matrix[0][0] << " | " << matrix[0][1] << " | " << matrix[0][2] << endl;

cout << "_____|_____|_____" << endl;
cout << " | | " << endl;

cout << " " << matrix[1][0] << " | " << matrix[1][1] << " | " << matrix[1][2] << endl;

cout << "_____|_____|_____" << endl;
cout << " | | " << endl;

cout << " " << matrix[2][0] << " | " << matrix[2][1] << " | " << matrix[2][2] << endl;

cout << " | | " << endl;

}
}

}

void TicTacToe::human_turns()
{
int a;
int row = 0;
int col = 0;
bool pos_is_valid = true;
char empty_field_val = ' ';
do {
cout << "Choose a number from the range of values" << endl;
cin >> a;

} while (a == matrix[row][col]);

switch (a)
{
case 1:row = 0; col = 0; empty_field_val = '1'; break;

case 2:row = 0; col = 1; empty_field_val = '2'; break;

case 3:row = 0; col = 2; empty_field_val = '3'; break;

case 4:row = 1; col = 0; empty_field_val = '4'; break;

case 5:row = 1; col = 1; empty_field_val = '5'; break;

case 6:row = 1; col = 2; empty_field_val = '6'; break;

case 7:row = 2; col = 0; empty_field_val = '7'; break;

case 8:row = 2; col = 1; empty_field_val = '8'; break;

case 9:row = 2; col = 2; empty_field_val = '9'; break;

default:pos_is_valid = false;
cout << "Number entered is invalid. Try again" << endl;
human_turns();
break;
}

if (pos_is_valid) {
if (matrix[row][col] == empty_field_val) {
matrix[row][col] = human_turn;

}
else {
cout << "Field is already in use. Try again" << endl;
CPU_turns();

}
}

}

void TicTacToe::CPU_turns()
{
while (true) {
Computer_turn = (rand() % 9) + 1;
int row = (Computer_turn - 1) / 3;
int col = (Computer_turn - 1) % 3;
char grid_pos = matrix[row][col];
if (grid_pos == 'X' || grid_pos == 'Y') {
continue;
}
else {
cout << "The computer has made a move" << Computer_turn << endl;
matrix[row][col] = 'O';
break;
}
}
}

void TicTacToe::Set_players()
{
//if player 1 can win on this move, block the move
if (human_turn == 'X') {
human_turn= 'O';
}
else {
human_turn = 'X';
}
if (Computer_turn == 'O') {
Computer_turn = 'X';
}
else {
Computer_turn = 'O';
}
}

char TicTacToe::Results()
{
//Results of the first player - Wins
if (matrix[0][0] == 'X' && matrix[0][1] == 'X' && matrix[0][2] == 'X') {
return human_turn;
}
else if (matrix[1][0] == 'X' && matrix[1][1] == 'X' && matrix[1][2] == 'X') {
return human_turn;
}
else if (matrix[2][0] == 'X' && matrix[2][1] == 'X' && matrix[2][2] == 'X') {
return human_turn;
}
else if (matrix[0][0] == 'X' && matrix[1][0] == 'X' && matrix[2][0] == 'X') {
return human_turn;
}

else if (matrix[0][0] == 'X' && matrix[1][0] == 'X' && matrix[2][0] == 'X') {
return human_turn;
}
else if (matrix[0][1] == 'X' && matrix[1][1] == 'X' && matrix[2][1] == 'X') {
return human_turn;
}
else if (matrix[0][2] == 'X' && matrix[1][2] == 'X' && matrix[2][2] == 'X') {
return human_turn;
}

else if (matrix[0][0] == 'X' && matrix[1][1] == 'X' && matrix[2][2] == 'X') {
return human_turn;
}
else if (matrix[2][0] == 'X' && matrix[1][1] == 'X' && matrix[0][2] == 'X') {
return human_turn;
}


//Second player result - Wins
if (matrix[0][0] == 'O' && matrix[0][1] == 'O' && matrix[0][2] == 'O') {
return Computer_turn;
}
else if (matrix[1][0] == 'O' && matrix[1][1] == 'O' && matrix[1][2] == 'O') {
return Computer_turn;
}
else if (matrix[2][0] == 'O' && matrix[2][1] == 'O' && matrix[2][2] == 'O') {
return Computer_turn;
}
else if (matrix[0][0] == 'O' && matrix[1][0] == 'O' && matrix[2][0] == 'O') {
return Computer_turn;
}

else if (matrix[0][0] == 'O' && matrix[1][0] == 'O' && matrix[2][0] == 'O') {
return Computer_turn;
}
else if (matrix[0][1] == 'O' && matrix[1][1] == 'O' && matrix[2][1] == 'O') {
return Computer_turn;
}
else if (matrix[0][2] == 'O' && matrix[1][2] == 'O' && matrix[2][2] == 'O') {
return Computer_turn;
}

else if (matrix[0][0] == 'O' && matrix[1][1] == 'O' && matrix[2][2] == 'O') {
return Computer_turn;
}
else if (matrix[2][0] == 'O' && matrix[1][1] == 'O' && matrix[0][2] == 'O') {
return Computer_turn;
}

return '/';


}
Appreciate it if you guys could reply to my message. Thank you

[Zaap]

As I said in the previous thread, please use [.code][./code] beacons. I can't even see where you're displaying the turn count.

[Ganado]

In your Run_game main loop, you sequentially call human_turns(); CPU_turns();.
But then within human_turns(), there is also logic to call CPU_turns(). Is this intended?

I would avoid recursion in this logic... I think it just makes things more confusing. (Prefer loops)

Edit: It looks like you posted almost the same topic twice. Please try not to double post.

[kabirdevramana96]

So how do I go about doing this? Making the code much more simpler.

[Ganado]

The most complicated function is your results function, but other than that, it doesn't seem too bad. Some of your switch logic could be refactored but that's not too important. I think the biggest thing is just stepping through the code's logic yourself and looking at where the CPU turn is called, since you're saying the issue is the CPU is going too often.

First, you should run your program in a debugger*, line by line. See what it's doing every line, and you'll be able to tell the point at which it makes the CPU go more often than the player. Only once you identify where the problem is can you start to figure out a solution.

* Debugging links:
https://docs.microsoft.com/en-us/visualstudio/debugger/quickstart-debug-with-cplusplus?view=vs-2019 (Visual Studio)
http://wiki.codeblocks.org/index.php/Debugging_with_Code::Blocks (Code Blocks)
http://eilat.sci.brooklyn.cuny.edu/cis1_5/HowToDebug.htm (Dev-C++)

[jonnin]

slowly, by looking at the code and improving it repeatedly until it feels clean and right.

lets take it iteratively.
you might make pass 1 and make a function that can check if someone won, regardless of X vs O. You can return X,O, or N (X won, O won, No one won) or something like that. That gets rid of a gigantic block of code that is identical apart from 'x' or 'o'.

you can also simplify the if-then logic by tons. I want you to try that without help first, but it can be cut way, way down in both space and complexity.

draw can also be condensed down to a much cleaner, smaller function.

you can also take a pass to remove magic numbers (consider named enums).

These things are overkill for a 2 page program, but you generally want to take a few passes over your code once it works to ensure it does not have a glaring performance problem (it does not have to be tweaked to the max, but nether should it take a dirt nap for 1/2 a second to iterate 20 things), that it is easy to read and understand and modify, and that it is cleanly broken into functions and blocks, etc.

[dhayden]

Please post code that can be compiled and run.

I think it will help you organize the code if you write comments above each method that says exactly what the method should do. Then make sure that the code does what you indent it to do. If you do this, you might question why human_turns() calls CPU_turns(). Or even why it calls human_turns() recursively.

What does it mean when human_turn=='X' and Computer_turn=='X' also? The answer, of course, is "well that's not legal." If it isn't legal, then try to encode the data so that it's impossible for that to happen. My advice is just one variable:
bool human_turn; // true means it's the human's turn. false means the computers.
See what I did there? This records whose turn it is while making it impossible to be ambiguous.

Choose your names carefully. What does TicTacToe::Results() really do? Hint: this method should return the state of the board, which is one of:
- human won
- computer won
- tie game
- nobody has won yet.

You can do this by returning a char or an enum.

[jonnin]

You can do this by returning a char or an enum.

the concept here is similar to boolean logic but instead of 2 states, you have 3 or more. This can go way off into weird math or finite state machine theory, but for this kind of code, just recognize that you have a few states and that you are in one of those, and that you are using a tiny piece of a bigger idea that you will want to learn at some point. Multi state can be used to greatly reduce work esp with lots of conditions around the state its in and are really powerful to get rid of tons of redundant conditions. This can be applied here, actually ...