Optimize this code

[brandon66]

Hey guys, im building a Tic Tac Toe game and im checking for a winner via this function(code below), Im juct wondering if there is a better way of doing this then how im doing it. if so could you point me in the right direction thanks :)

The array to begin with is initialized with an asterisk for each of its elements.

int checkBoard(const char array[][COLSIZE], const int rowSize) {
    //variables
    int i = 0;
    char temp;
    int winner = 5;

    for (i = 0; i < rowSize; i++) { // checking the rows
        if (array[i][0] == array[i][1] && array[i][0] == array[i][2]) {
            temp = array[i][0];
        }
    }
    for (i = 0; i < COLSIZE; i++) { // checking the columns 
        if (array[0][i] == array[1][i] && array[0][i] == array[2][i]) {
            temp = array[0][i];
        }
    }
    // This will check diagonals 
    if (array[0][0] == array[1][1] && array[1][1] == array[2][2]) {
        temp = array[0][0];
    }
    if (array[0][2] == array[1][1] && array[1][1] == array[2][0]) {
        temp = array[0][2];
    }


    if (temp == 'X') {
        //player one wins
        winner = 1;
    } else if (temp == 'O') {
        //player 2 wins
        winner = 2;
    } else if (temp != 'X' && temp != 'O') {
        //check if board is completed
        if (array[0][0] != '*' && array[0][1] != '*' && array[0][2] != '*' &&
            array[1][0] != '*' && array[1][1] != '*' && array[1][2] != '*' &&
            array[2][0] != '*' && array[2][1] != '*' && array[2][2] != '*') {
            //game is complete
            winner = 3;
        } else {
            //game not complete
            winner = -1;
        }

    }
    return winner;
}

[vmanes]

I think the test in line 32 is unneccessary. If either X or O had won, you won't get to that test. If neither has won, then you should do the completed game test as the else action.

Initialize the winner variable to the not complete value. When the if..else if. else block is done, either a winner or complete game value will be set, or the not complete value will remain.

In line 3, the temp variable should be initialized to something, other than X or O, perhaps a blank space?

[tinstaafl]

A few things to consider:

Try to avoid keywords for object/variable names and use something more apropos You may know what it refers to now, but will you remember months or years from now.

Using an enum for the 4 conditions(No winner, X, O, and Draw) set to the char values allows you to cast the winner instead of extra conditionals. To read the enum value just cast it to char.

Moving your checks for rows and columns into one loop simplifies your code and eliminates the extra loop.

Combining the 2 diagonal conditionals into 1 and using the center as the one to cast if either is the winner, simplifies your code quite a bit:

Since the row size and the column size are the same and only one loop is used, I eliminated rowsize from the argument list.

#include <iostream>

using namespace std;
const int COLSIZE = 3;
enum Winner
{
    N = 78,
    X = 88,
    O = 79,
    D = 68
};
Winner checkBoard(const char board[][COLSIZE])
{
    Winner won = N;
    bool completeboard = true;
    //Check diagonals
    if((board[1][1] == board[0][0] && board[1][1] == board[2][2]) || (board[1][1] == board[2][0] && board[1][1] == board[0][2]))
        won = (Winner)board[1][1];
    else
        //Check rows and columns
        for(int i = 0;i < COLSIZE;i++)
        {
            if(board[i][0] == '*' || board[i][1] == '*' || board[i][2] == '*' || board[0][i] == '*' || board[1][i] == '*' || board[2][i] == '*')
                completeboard = false;
            if(board[i][0] != '*' && board[i][0] == board[i][1] && board[i][0] == board[i][2])
            {
                won = (Winner)(board[i][0]);
                break;
            }
            if(board[0][i] != '*' && board[0][i] == board[1][i] && board[0][i]== board[2][i])
            {
                won = (Winner)(board[0][i]);
                break;
            }
        }
    //Determine if it's a draw
    if(completeboard && won == N)
        won = D;
    return won;
}
int main()
{
    char board[3][3] = {{'O','X','X'},{'X','O','X'},{'O','X','O'}};
    Winner thewinner = checkBoard(board,3);
    //For this board the winner value is 'O' 79
    cout << (char)thewinner;
    return 0;
}

This code is based on the assumption that the board will be checked after each player makes a move. Therefore only the first winning condition, that is found, is recognized.

[tinstaafl]

You're very welcome. If you have no other concerns, please remember to mark this solved. Thanks.

[brandon66]

I tried implementing the the code, but i cant seem to get it to run correctly.

#include <iostream>
#include <iomanip>
#include <stdlib.h>

using namespace std;

//global constants
const int PLAYER1 = 1; //Player 1
const int PLAYER2 = 2;//player 2
const int CHOICE1 = 1;//menu choice 1
const int CHOICE2 = 2;//menu choice 2
const int CHOICE3 = 3;//menu choice 3
const int ROWSIZE = 3;//number of rows
const int COLSIZE = 3;//number of columns
enum Winner //enumerated type for winner
{
    N = 78,//game not complete
    X = 88,//'X' wins
    O = 79,//'O' wins
    D = 68//game is a draw
};

//function prototypes
void intro();
int menu(const int choice1,const int choice2, const int choice3);
void printGameBoard(const char array[][COLSIZE], const int ROWSIZE);
void getPlayerInput(char array[][COLSIZE], const int ROWSIZE, const int player);
Winner checkBoard(const char array[][COLSIZE]);
/*
 * Main
 */
int main() {
    //variables
    int choice; //users menu choice
    int winner = 0;
    char gameBoard[ROWSIZE][COLSIZE] = {{'*','*','*'},//array for game board
                                        {'*','*','*'},//initialized with *
                                        {'*','*','*'}}; 

    //intro
    intro();
    //show menu
    choice = menu(CHOICE1,CHOICE2,CHOICE3);

    if(choice == CHOICE1){
    //play the game

   while(!winner){ 
        //check the board
    Winner winner = checkBoard(gameBoard);
     cout << winner;
     // clear the screen
     system("cls");
    //print the board
    printGameBoard(gameBoard,ROWSIZE);   
    //get player 1 input
    getPlayerInput(gameBoard,ROWSIZE, PLAYER1);
    //redraw board
    system("cls");
    printGameBoard(gameBoard, ROWSIZE);
    winner = checkBoard(gameBoard);

    //get player 2 input
    getPlayerInput(gameBoard,ROWSIZE, PLAYER2);
    //redraw board
    system("cls");
    printGameBoard(gameBoard, ROWSIZE);
    winner = checkBoard(gameBoard);

    }
    //print winner

    }else if (choice == CHOICE2){
    //show the game instructions

    }else if (choice == CHOICE3){
     //exit the program
     cout << "Quitting" << endl;
    }

    return 0;
}

/*
 * Function: intro
 * Description: Displays an intro message to the user
 */
void intro() {
    //Program introduction
    cout << "*****************************************" << endl
            << "** 2 Player Game of Tic-Tac-Toe        ** \n"
            << "** Created by: Brandon                 ** \n"
            << "** 2-Dimensional Arrays                **" << endl
            << "**                                     **" << endl
            << "*****************************************" << endl << endl;
}

/*
 * Function: menu
 * Description: Displays a menu to the user and asks the user
 * to select a choice. it then return the choice as an integer.
 */
int menu(const int choice1,const int choice2, const int choice3){

    //variables
    int choice;

    //display a menu and return users choice
    cout << "\t Tic-Tac-Toe Menu" << endl << endl
         << "1. Play Game" << endl
         << "2. Game Instructions" << endl
         << "3. Exit" << endl << endl
         << "Enter your choice: ";
    cin >> choice;

    //check the users input
    //do loop while users choice does not equal a menu choice
    //if equals a menu choice return users choice
    do{
    if(choice >= 1 && choice <= 3){
        return choice;
    }else{ 
        cout << "Choice invalid try again." << endl
             << "Enter your choice: ";
        cin >> choice; 
         }
     }while(choice != 1 || choice != 2 || choice != 3);  
 }

/*
 * Function: printGameBoard
 * Description: Use a nested loop to print the 2D array
 */
void printGameBoard(const char array[][COLSIZE], const int rowSize) {
    //print the game board
    cout    << setw(30) << "Tic-Tac-Toe" << endl << endl << setw(5) << " "
            << "* Player 1 is (X) and Player 2 is (O)  *"<< endl << endl;
    //nested loop to loop the rows/columns
    for (int row = 0; row < rowSize; row++) {
        cout<<endl;
        for (int column = 0; column < COLSIZE; column++) {
            cout << setw(8) << " " << setw(4)<< array[row][column];
        }
        cout << endl << endl;
    }
} 

/*
 * Function: getPlayerInput
 * Description: Gets the players input and stores it 
 * into the array.
 */
void getPlayerInput(char array[][COLSIZE], const int size, const int player){
    //variables
    int tempRow;
    int tempCol;

    if(player == 1){
        //get input
        cout << "Player " << player << " enter the row position(0-2): ";
        cin >> tempRow;
        cout << "Player " << player << " enter the column position(0-2: ";
        cin >> tempCol;

        array[tempRow][tempCol] = 'X';
    }else if (player == 2){
        //get input
        cout << "Player " << player << " enter the row position(0-2): ";
        cin >> tempRow;
        cout << "Player " << player << " enter the column position(0-2: ";
        cin >> tempCol;

        array[tempRow][tempCol] = 'O';
    }

}
/*
 * Function: checkBoard
 * Description: check the game board for a winner
 * dont need to send row size over since rowsize = columnsize
 */
Winner checkBoard(const char board[][COLSIZE]) {
    //enum type   
    Winner won = N;

    bool completeboard = true;
    //Check diagonals
    if((board[1][1] == board[0][0] && board[1][1] == board[2][2]) || (board[1][1] == board[2][0] && board[1][1] == board[0][2]))
        won = (Winner)board[1][1];
    else
        //Check rows and columns
        for(int i = 0;i < COLSIZE;i++)
        {
            if(board[i][0] == '*' || board[i][1] == '*' || board[i][2] == '*' || board[0][i] == '*' || board[1][i] == '*' || board[2][i] == '*')
                completeboard = false;
            if(board[i][0] != '*' && board[i][0] == board[i][1] && board[i][0] == board[i][2])
            {
                won = (Winner)(board[i][0]);
                break;
            }
            if(board[0][i] != '*' && board[0][i] == board[1][i] && board[0][i]== board[2][i])
            {
                won = (Winner)(board[0][i]);
                break;
            }
        }
    //Determine if it's a draw
    if(completeboard && won == N)
        won = D;

    return won;
}

[tinstaafl]

Saying it doesn't work right doesn't tell anybody much. Exactly what is it doing wrong?

[brandon66]

sorry. It just keeps asking for the users inputs even if a player has won, Its like checkBoard isnt even getting called.

[tinstaafl]

Sorry had a small bug in my original code, forgot to check if board[1][1] was an *. Fixed that. Also I changed the players to Player X and Player O. This way they can be Winner types which simplifies parts of the code quite a bit. One thing I noticed you had no check for wrong input from the user when choosing the board position to play, so I added that as well. Whenever you're outputting literal strings to the console try to use the newline character('\n') instead of endl. It's more efficient that way.

Here's the modified code:

#include <iostream>
#include <iomanip>
#include <stdlib.h>
using namespace std;
//global constants
enum Winner //enumerated type for winner
{
    N = 78,//game not complete
    X = 88,//'X' wins
    O = 79,//'O' wins
    D = 68//game is a draw
};
const Winner PLAYER1 = X; //Player 1
const Winner PLAYER2 = O;//player 2
const int CHOICE1 = 1;//menu choice 1
const int CHOICE2 = 2;//menu choice 2
const int CHOICE3 = 3;//menu choice 3
const int ROWSIZE = 3;//number of rows
const int COLSIZE = 3;//number of columns

//function prototypes
void intro();
int menu(const int choice1,const int choice2, const int choice3);
void printGameBoard(const char array[][COLSIZE], const int ROWSIZE);
void getPlayerInput(char array[][COLSIZE], const Winner player);
Winner checkBoard(const char array[][COLSIZE]);
/*
* Main
*/
int main() {
    //variables
    int choice; //users menu choice
    Winner winner = N;
    char gameBoard[ROWSIZE][COLSIZE] = 
    {{'*','*','*'},//array for game board
    {'*','*','*'},//initialized with *
    {'*','*','*'}};
    //intro
    intro();
    //show menu
    choice = menu(CHOICE1,CHOICE2,CHOICE3);
    if(choice == CHOICE1){
        //play the game
        while(winner == N){
            //check the board
            winner = checkBoard(gameBoard);
            cout << (char)winner;
            cin.get();
            // clear the screen
            system("cls");
            //print the board
            printGameBoard(gameBoard,ROWSIZE);
            //get player 1 input
            getPlayerInput(gameBoard, PLAYER1);
            //redraw board
            system("cls");
            printGameBoard(gameBoard, ROWSIZE);
            winner = checkBoard(gameBoard);
            if(winner != N)
                break;
            //get player 2 input
            getPlayerInput(gameBoard, PLAYER2);
            //redraw board
            system("cls");
            printGameBoard(gameBoard, ROWSIZE);
            winner = checkBoard(gameBoard);
        }
        //print winner
        if(winner == D)
            cout << "Oops you played to a draw.\n";
        else
            cout <<"Congratulations Player " << (char)winner << " won!!!!\n";
        cin.get();
    }else if (choice == CHOICE2){
        //show the game instructions
    }else if (choice == CHOICE3){
        //exit the program
        cout << "Quitting\n";
    }
    cin.get();
    return 0;
}
/*
* Function: intro
* Description: Displays an intro message to the user
*/
void intro() {
    //Program introduction
    cout << "*****************************************\n"
        << "** 2 Player Game of Tic-Tac-Toe ** \n"
        << "** Created by: Brandon ** \n"
        << "** 2-Dimensional Arrays **\n"
        << "** **\n"
        << "*****************************************\n\n";
}
/*
* Function: menu
* Description: Displays a menu to the user and asks the user
* to select a choice. it then return the choice as an integer.
*/
int menu(const int choice1,const int choice2, const int choice3){
    //variables
    int choice;
    //display a menu and return users choice
    cout << "\t Tic-Tac-Toe Menu\n\n"
        << "1. Play Game\n"
        << "2. Game Instructions\n"
        << "3. Exit\n\n"
        << "Enter your choice: ";
    cin >> choice;
    //check the users input
    //do loop while users choice does not equal a menu choice
    //if equals a menu choice return users choice
    do{
        if(choice >= 1 && choice <= 3){
            return choice;
        }else{
            cout << "Choice invalid try again.\n"
                << "Enter your choice: ";
            cin >> choice;
        }
    }while(choice != 1 || choice != 2 || choice != 3);
    return choice;
}
/*
* Function: printGameBoard
* Description: Use a nested loop to print the 2D array
*/
void printGameBoard(const char array[][COLSIZE], const int rowSize) {
    //print the game board
    cout << setw(30) << "Tic-Tac-Toe\n\n" << setw(5) << " "
        << "* Player 1 is (X) and Player 2 is (O) *n\n";
    //nested loop to loop the rows/columns
    for (int row = 0; row < rowSize; row++) {
        cout<<endl;
        for (int column = 0; column < COLSIZE; column++) {
            cout << setw(8) << " " << setw(4)<< array[row][column];
        }
        cout << endl << endl;
    }
}
/*
* Function: getPlayerInput
* Description: Gets the players input and stores it
* into the array.
*/
void getPlayerInput(char array[][COLSIZE], const Winner player){
    //variables
    bool done = false;
    int tempRow;
    int tempCol;
    while(!done)
    {
        //get input
        cout << "Player " << (char)player << " enter the row position(0-2): ";
        cin >> tempRow;
        cout << "Player " << (char)player << " enter the column position(0-2): ";
        cin >> tempCol;
        if(tempRow < COLSIZE && tempCol < COLSIZE && array[tempRow][tempCol] == '*')
        {
            array[tempRow][tempCol] = (char)player;
            done = true;
        }
    }
}
/*
* Function: checkBoard
* Description: check the game board for a winner
* dont need to send row size over since rowsize = columnsize
*/
Winner checkBoard(const char board[][COLSIZE]) {
    //enum type
    Winner won = N;
    bool completeboard = true;
    //Check diagonals
    if(board[1][1] != '*' && ((board[1][1] == board[0][0] && board[1][1] == board[2][2]) || (board[1][1] == board[2][0] && board[1][1] == board[0][2])))
        won = (Winner)board[1][1];
    else
        //Check rows and columns
        for(int i = 0;i < COLSIZE;i++)
        {
            if(board[i][0] == '*' || board[i][1] == '*' || board[i][2] == '*' || board[0][i] == '*' || board[1][i] == '*' || board[2][i] == '*')
                completeboard = false;
            if(board[i][0] != '*' && board[i][0] == board[i][1] && board[i][0] == board[i][2])
            {
                won = (Winner)(board[i][0]);
                break;
            }
            if(board[0][i] != '*' && board[0][i] == board[1][i] && board[0][i]== board[2][i])
            {
                won = (Winner)(board[0][i]);
                break;
            }
        }
        //Determine if it's a draw
        if(completeboard && won == N)
            won = D;
        return won;
}

[brandon66]

Ahh i missed that. Also yes i did not stick the check for wrong input in there, I waned to get the checkBoard completed first, but that saves me some time. Thanks alot for your help! I might be back with some more C++ questions as i continue learning it, so ill look you up! thanks.