Multi tool use

Tic tac toe C++ overwriting the board

Hi so I have it set up the way I want to for the most part. The last part I'm trying to implement is to have it so the input does not overwrite itself when input either Xor 0. If someone could take a look at it and tell me what exactly I did wrong with my noRepeat function I would greatly appreciate it. Thanks.

X

0

#include <iostream>

using namespace std;
const int ROWS = 3;
const int COLS = 3;
char Player1 = 'X';
char Player2 = 'O';
int row, col;
char board[3][3]= { '*', '*', '*', '*', '*', '*', '*', '*', '*'}; //Game board


//Prototypes

void noRepeat();
void drawBoard();
void selection(int&, int&);
char winner();
void switchPlayer();

int main()
{
cout << "Tic Tac Toe!" << endl << endl;

drawBoard();
cout << endl << endl;
while (1)
{

if ( noRepeat() )
{
selection(row, col);
drawBoard();
switchPlayer();
}
if (winner() == 'X')
{
cout << "Player 1 wins!" << endl;
break;
}
else if (winner() == 'O')
{
cout << "Player 2 wins!" << endl;
break;
}

}


return 0;
}


// input function
void selection(int &row, int &col)
{
cout << "Enter a number for row between 0 - 2" << endl;
cin >> row;
if (row < 0 || row > 2) {
cout << "Invalid selection select a row between 0 - 2" << endl;
cin >> row;
}

cout << "Enter a number for col between 0 - 2" << endl;
cin >> col;
if (col < 0 || col > 2) {
cout << "Invalid selection select a row between 0 - 2" << endl;
cin >> col;
}



if (row == 0 && col == 0)
board[0][0] = Player1;
else if (row == 0 && col == 1)
board[0][1] = Player1;
else if (row == 0 && col == 2)
board[0][2] = Player1;
else if (row == 1 && col == 0)
board[1][0] = Player1;
else if (row == 1 && col == 1)
board[1][1] = Player1;
else if (row == 1 && col == 2)
board[1][2] = Player1;
else if (row == 2 && col == 0)
board[2][0] = Player1;
else if (row == 2 && col == 1)
board[2][1] = Player1;
else if (row == 2 && col == 2)
board[2][2] = Player1;


}


//Board function
void drawBoard() {
for (int i = 0; i < ROWS; i++) {

for (int j = 0; j < COLS; j++) {
cout << board[i][j];
}
cout << endl;
}
}


//Function to switch between X's and O's

void switchPlayer() {

char temp = Player1;
if (Player1) {
Player1 = Player2;
}

if (Player2)
{
Player2 = temp;
}
}


//conditional winner checker
char winner() {
if (board[0][0] == 'X' && board[0][1] == 'X' && board[0][2] == 'X')
return 'X';
if (board[1][0] == 'X' && board[1][1] == 'X' && board[1][2] == 'X')
return 'X';
if (board[2][0] == 'X' && board[2][1] == 'X' && board[2][2] == 'X')
return 'X';

if (board[0][0] == 'X' && board[1][0] == 'X' && board[2][0] == 'X')
return 'X';
if (board[0][1] == 'X' && board[1][1] == 'X' && board[2][1] == 'X')
return 'X';
if (board[0][2] == 'X' && board[1][2] == 'X' && board[2][2] == 'X')
return 'X';

if (board[0][0] == 'X' && board[1][1] == 'X' && board[2][2] == 'X')
return 'X';
if (board[2][0] == 'X' && board[1][1] == 'X' && board[0][2] == 'X')
return 'X';

//Player 2
if (board[0][0] == 'O' && board[0][1] == 'O' && board[0][2] == 'O')
return 'O';
if (board[1][0] == 'O' && board[1][1] == 'O' && board[1][2] == 'O')
return 'O';
if (board[2][0] == 'O' && board[2][1] == 'O' && board[2][2] == 'O')
return 'O';

if (board[0][0] == 'O' && board[1][0] == 'O' && board[2][0] == 'O')
return 'O';
if (board[0][1] == 'O' && board[1][1] == 'O' && board[2][1] == 'O')
return 'O';
if (board[0][2] == 'O' && board[1][2] == 'O' && board[2][2] == 'O')
return 'O';

if (board[0][0] == 'O' && board[1][1] == 'O' && board[2][2] == 'O')
return 'O';
if (board[2][0] == 'O' && board[1][1] == 'O' && board[0][2] == 'O')
return 'O';
}


//Function to prevent overwriting.
void noRepeat() {
for (int i = 0; i < ROWS; i++) {
for (int j = 0; j < COLS; j++) {
if (board[i][j] == Player1 || board[i][j] == Player2) {
cout << "Invalid selection, please choose a row and column
again."
}
}
}
}


if ( noRepeat() )

noRepeat

void

winner()

'X'

'O'

3 Answers
3

Your function noRepeat() returns void. But must return boolean type.

noRepeat()

void

boolean

(1) When a player select Row/Col, it necessary to check that the array has '*' char in that selection (i.e. unused yet). (2) Add an int variable 'selections' and on any valid selection increment it by 1. When selection hits 9, the board is full. Like that, you don't need noRepeat() at all.

noRepeat()

The noRepeat() function just print but return void. So what is the purpose of asking if (noRepeat())?

noRepeat()

if (noRepeat())

The function switchPlayer() does nothing. This one below, for example, if you pass the "current player", returns the other one: ..Use: (1) Add char currentPalayer = Player1; variable, and (2) currentPalayer = switchPlayer(currentPalayer); when you want to switch player. Use the currentPalayer variable, when Player1 and Player2 can be const char of #define

switchPlayer()

char currentPalayer = Player1;

currentPalayer = switchPlayer(currentPalayer);

currentPalayer

Player1

Player2

const char

#define

char switchPlayer(char currentPlayer) {

const int K = Player1 + Player2;

return (char) (K - currentPlayer);
}


I guess there are more issues.

BTW: Cut to 1/2 the winner() function:

//conditional winner checker
char winner() {
if (board[0][0] == board[0][1] && board[0][1] == board[0][2])
return board[0][0];
if (board[1][0] == board[1][1] && board[1][1] == board[1][2])
return board[1][0];
if (board[2][0] == board[2][1] && board[2][1] ==board[2][2]))
return board[2][0];

if (board[0][0] == board[1][0] && board[1][0] == board[2][0])
return board[0][0] ;
if (board[0][1] == board[1][1] && board[1][1] == board[2][1])
return board[0][1];
if (board[0][2] == board[1][2] && board[1][2] == board[2][2])
return board[0][2];

if (board[0][0] == board[1][1] && board[1][1] == board[2][2])
return board[0][0];
if (board[2][0] == board[1][1] && board[1][1] == board[0][2])
return board[2][0];
}


We can all suggest improvements to your code and perhaps it should be handled over at https://codereview.stackexchange.com/. Short of doing a major recode, let's look at doing minor tweaks to one of your functions:

void selection(int &row, int &col)
{
cout << "Enter a number for row between 0 - 2" << endl;
cin >> row;
while (row < 0 || row > 2) {
cout << "Invalid selection select a row between 0 - 2" << endl;
cin >> row;
}

cout << "Enter a number for col between 0 - 2" << endl;
cin >> col;
while (col < 0 || col > 2) {
cout << "Invalid selection select a row between 0 - 2" << endl;
cin >> col;
}

board[row][col] = Player1;
}


What I have done to your function was change the validation logic from if to while loop since you want to keep asking until you received a valid input. The other thing was I refactor away your setter. Why have all those if variations when a single 1-liner board[row]col] = Player; can do the same job?

if

while

if

board[row]col] = Player;

Overall, I didn't like this function because there's a missed opportunity to do validation for '*' here. I cannot demonstrate that without rewriting a larger portion of that function.

'*'

Other issues I had with the code was:

int currentPlayer

char playerMarker[2] = { 'X', 'O' };

currentPlayer = 1 - currentPlayer;

char board[ROW][COL + 2];

puts(board[0]); puts(board[1]); puts(board[2]);

noRepeat

By clicking "Post Your Answer", you acknowledge that you have read our updated terms of service, privacy policy and cookie policy, and that your continued use of the website is subject to these policies.