I have reached a mini-capstone where I am asked to create a tic tac toe program. I have successfully created the program. What I'd like feedback on is two things: 1. How do I number my columns like I numbered my rows? 2. Can you spot areas where this code could be written in a more optimal manner (i.e. better code reuse, less processing needed, less code, etc).

// this program simulates a tic tac toe game between two players

#include <iostream>
#include <cstdlib> // for exit function
using namespace std;

// global constants
const int COL = 3;

// function prototypes
void displayBoard(char[][COL], int);
void player1Turn(char[][COL], int);
void player2Turn(char[][COL], int);
void player1Winner(char[][COL], int);
void player2Winner(char[][COL], int);

int main()
{
    const int ROW = 3;
    char gameBoard[ROW][COL];

    // initialize gameBoard with asterisks to represent blank spots
    for (int x = 0; x < ROW; x++)
    {
        for (int y = 0; y < COL; y++)
        {
            gameBoard[x][y] = '*';
        }
    }

    // flag to determine if game is still in progress
    bool stillPlaying = true;
    const int maxTurns = 9; // maximum number of turns in a game of tic tac toe
    int turnNum = 0; // current turn number

    do
    {
        // display gameBoard
        cout << endl;
        displayBoard(gameBoard, ROW);

        // determine if player 2 has won
        player2Winner(gameBoard, ROW);

        // let player 1 make their move
        player1Turn(gameBoard, ROW);

        // increment turnNum
        turnNum++;

        // display gameBoard
        cout << endl;
        displayBoard(gameBoard, ROW);

        // determine if player 1 has won
        player1Winner(gameBoard, ROW);

        // player 1 will always have the last turn if no winner has been determined by then
        // if numTurn is equal to maxTurns after player 1's turn, then the game is over
        if (turnNum == maxTurns)
        {
            stillPlaying = false;
            continue;
        }

        // let player 2 make their move
        player2Turn(gameBoard, ROW);

        // increment turnNum
        turnNum++;

    } while (stillPlaying);

    cout << "\nThe game is a tie.\n";
}

/*
    function definition displayBoard
    displayBoard displays an updated Tic Tac Toe game board that show empty positions as well
    as the positions occupied by players 1 and 2.
*/

void displayBoard(char gameBoard[][COL], int row)
{
    for (int x = 0; x < row; x++)
    {
        cout << "Row " << x + 1 << "\t";
        for (int y = 0; y < COL; y++)
        {
            cout << gameBoard[x][y] << "   "; // place 3 spaces inbetween each column for readability
        }
        cout << endl; // place a blank line between each row
    }
}

/*
    function definition player1Turn
    player1Turn takes X as it's input and places it on the gameBoard
*/

void player1Turn(char gameBoard[][COL], int row)
{
    int rowNum,
        colNum;
    do
    {
        cout << "\nIt is Player 1's turn.\n"
            << "Select the row number where you wish to place your X: ";
        cin >> rowNum;
        rowNum = rowNum - 1;
        cout << "Select the column number where you wish to place your X: ";
        cin >> colNum;
        colNum = colNum - 1;

        // input validation: do not allow players to move in occupied positions
        if (gameBoard[rowNum][colNum] != '*')
        {
            cout << "You may only place an X in blank positions. Please try again.\n";
        }

    } while (gameBoard[rowNum][colNum] != '*');

    gameBoard[rowNum][colNum] = 'X';
}

/*
    function definition player2Turn
    player2Turn takes O as it's input and places it on the gameBoard
*/

void player2Turn(char gameBoard[][COL], int row)
{
    int rowNum,
        colNum;
    do
    {
        cout << "\nIt is Player 2's turn.\n"
            << "Select the row number where you wish to place your O: ";
        cin >> rowNum;
        rowNum = rowNum - 1; // decrement row number so it corresponds to array
        cout << "Select the column number where you wish to place your O: ";
        cin >> colNum;
        colNum = colNum - 1; // decrement column number so it corresponds to array

        // input validation: do not allow players to move in occupied positions
        if (gameBoard[rowNum][colNum] != '*')
        {
            cout << "You may only place an O in blank positions. Please try again.\n";
        }

    } while (gameBoard[rowNum][colNum] != '*');

    gameBoard[rowNum][colNum] = 'O';
}

/*
    function definition player1Winner
    player1Winner checks the gameBoard to determine if there are any instances where player 1
    has 3 X's in a row, column, or angle.
*/

void player1Winner(char gameBoard[][COL], int row)
{
    double total; // accumulator
    bool winner = false; // flag

    // determine if there are 3 X's in a row
    for (int x = 0; x < row; x++)
    {
        // reset accumulator per row
        total = 0;

        for (int y = 0; y < COL; y++)
        {
            if (gameBoard[x][y] == 'X')
            {
                total++;
            }
        }
        if (total == 3)
            winner = true;
    }

    // determine if there are 3 X's in a column
    for (int y = 0; y < COL; y++)
    {
        // reset accumulator per column
        total = 0;

        for (int x = 0; x < row; x++)
        {
            if (gameBoard[x][y] == 'X')
            {
                total++;
            }
        }
        if (total == 3)
            winner = true;
    }

    // reset accumulator
    total = 0;
    // determine if there are 3 X's in the downward angle
    for (int i = 0; i < row; i++)
    {
        if (gameBoard[i][i] == 'X')
            total++;
    }
    if (total == 3)
        winner = true;

    // reset accumulator
    total = 0;
    // determine if there are 3 X's in the upward angle
    int x = 0,
        y = 2;
    for (x, y; x < row; x++, y--)
    {
        if (gameBoard[x][y] == 'X')
            total++;
    }
    if (total == 3)
        winner = true;

    if (winner)
    {
        cout << "\nPlayer 1 is the winner!\n";
        exit(0);
    }
}

/*
    function definition player2Winner
    player2Winner checks the gameBoard to determine if there are any instances where player 2
    has 3 O's in a row, column, or angle.
*/

void player2Winner(char gameBoard[][COL], int row)
{
    double total; // accumulator
    bool winner = false; // flag

                         // determine if there are 3 X's in a row
    for (int x = 0; x < row; x++)
    {
        // reset accumulator per row
        total = 0;

        for (int y = 0; y < COL; y++)
        {
            if (gameBoard[x][y] == 'O')
            {
                total++;
            }
        }
        if (total == 3)
            winner = true;
    }

    // determine if there are 3 X's in a column
    for (int y = 0; y < COL; y++)
    {
        // reset accumulator per column
        total = 0;

        for (int x = 0; x < row; x++)
        {
            if (gameBoard[x][y] == 'O')
            {
                total++;
            }
        }
        if (total == 3)
            winner = true;
    }

    // reset accumulator
    total = 0;
    // determine if there are 3 X's in the downward angle
    for (int i = 0; i < row; i++)
    {
        if (gameBoard[i][i] == 'O')
            total++;
    }
    if (total == 3)
        winner = true;

    // reset accumulator
    total = 0;
    // determine if there are 3 X's in the upward angle
    int x = 0,
        y = 2;
    for (x, y; x < row; x++, y--)
    {
        if (gameBoard[x][y] == 'O')
            total++;
    }
    if (total == 3)
        winner = true;

    if (winner)
    {
        cout << "\nPlayer 2 is the winner!\n";
        exit(0);
    }
}
  • 1
    It's off-topic for this site, but the answer to your first question is something like std::cout << "Cols\t";for (int y = 1; y <= COL; y++) std::cout << y << " ";std::cout << std::endl; – mdfst13 Nov 22 '16 at 2:29
up vote 1 down vote accepted

Overall, this code is really easy to read and understand. Nice work!

Don't Repeat Yourself

The biggest issue I see is that you have 2 copies of several functions to deal with player 1 and player 2. It would be best to factor out the parts that are the same into a single function and pass in which player you're checking on. I would get rid of player1Turn() and player2Turn() and replace them with a single playerTurn() function. It would probably look something like this:

void playerTurn(char gameBoard[][COL], const int row, const int player)
{
    int rowNum,
        colNum;
    char playerMark;

    if (player == 1)
    {
        playerMark = 'X';
    }
    else
    {
        playerMark = 'O';
    }

    do
    {
        cout << "\nIt is Player " << player << "'s turn.\n"
            << "Select the row number where you wish to place your " << playerMark << ": ";
        cin >> rowNum;
        rowNum = rowNum - 1;
        cout << "Select the column number where you wish to place your " << playerMark << ": ";
        cin >> colNum;
        colNum = colNum - 1;

        // input validation: do not allow players to move in occupied positions
        if (gameBoard[rowNum][colNum] != '*')
        {
            cout << "You may only place an " << playerMark << " in blank positions. Please try again.\n";
        }

    } while (gameBoard[rowNum][colNum] != '*');

    gameBoard[rowNum][colNum] = playerMark;
}

Now you just call playerTurn(gameBoard, row, 1); for player 1 and playerTurn(gameBoard, row, 2); for player 2.

Likewise, I'd get rid of player1Winner() and player2Winner() and just make a playerWinner() where you pass in the number of the player you wish to check.

Keep Exits Minimal and Close Together

I was a little surprised by the fact that the player*Winner() functions would call exit(). For a small program like this, it's not a huge problem, but in a larger application it would make it difficult to figure out why an application exited. It might make more sense to have your function return a bool and then have the main() function determine from that result whether it should exit or not. So main() would end up looking something like this:

do
{
    // display gameBoard
    cout << endl;
    displayBoard(gameBoard, ROW);

    // determine if player 2 has won
    if (playerWinner(gameBoard, ROW, 2))
    {
        exit(0);
    }

    // let player 1 make their move
    playerTurn(gameBoard, ROW, 1);

    // increment turnNum
    turnNum++;

    // display gameBoard
    cout << endl;
    displayBoard(gameBoard, ROW);

    // determine if player 1 has won
    if (playerWinner(gameBoard, ROW, 1))
    {
        exit(0);
    }

... etc.

Performance

Since you asked about performance - I wouldn't expect performance to matter too much in this program. Your program stops for input after every function, and it will end up waiting there far longer than the time it will take to execute any of your functions.

But I do see a few ways you could make your code more efficient at the risk of making it slightly harder to read. In your playerWinner() function you could combine the checking of rows and columns into a single loop. Something like this:

double rowTotal; // accumulator
double colTotal;
bool winner = false; // flag

                     // determine if there are 3 X's in a row or column
for (int x = 0; x < row; x++)
{
    // reset accumulator per row
    rowTotal = 0;
    colTotal = 0;

    for (int y = 0; y < COL; y++)
    {
        if (gameBoard[x][y] == playerMark)
        {
            rowTotal++;
        }
        if (gameBoard[y][x] == playerMark)
        {
            colTotal++;
        }
    }
    if ((rowTotal == 3) || (colTotal == 3))
        winner = true;
}

Your Answer

 

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.

Not the answer you're looking for? Browse other questions tagged or ask your own question.