5
\$\begingroup\$

The exercise is from Alex Allain's book "Jumping into C++". From Chapter 10 Practical Problem 4:

  1. Write a small tic-tac-toe program that allows two players to play tic-tac-toe competitively. Your program should check to see if either player has won, or if the board is filled completely (with the game ending in a tie). Bonus: can you make your program detect if the game cannot be won by either side before the entire grid is filled?

Here's my code. My main goal was a functioning tick-tack-toe game with little concern on the user experience (e.g.: the coordinates a player enters start from 0, or (0,0) coordinate is in the upper left corner no lower left as one would intuitively think). As for coding style I wanted the code to be as DRY as possible, with small, atomic, easy to understand functions doing most of the work.

#include <iostream>

using namespace std;

bool checkWin(int board[][3], int player) {
    // horizontal
    for (int i = 0; i < 3; i++) {
        if (board[i][0] == player && board[i][1] == player && board[i][2] == player) return true;
    }

    // vertical
    for (int i = 0; i < 3; i++) {
        if (board[0][i] == player && board[1][i] == player && board[2][i] == player) return true;
    }

    // diagonal
    if ((board[0][0] == player && board[1][1] == player && board[2][2] == player) ||
        (board[0][2] == player && board[1][1] == player && board[2][0] == player)) 
        return true;

    return false;
}

// TODO implement function if the game is a tie before the last move

bool fillPlace(int board[][3], int x, int y, int value) {
    if (x > 2 || x < 0) {
        cout << "Invalid x coordinate. Try again.\n";
        return false;
    } else if (y > 2 || y < 0) {
        cout << "Invalid y coordinate. Try again.\n";
        return false;
    } else if (board[x][y] != 0) {
        cout << "Place is already filled. Try again.\n";
        return false;
    } else {
        board[x][y] = value;
        return true;
    }
}

void initBoard(int board[][3], int size) {
    for (int i = 0; i < size; i++) {
        for (int j = 0; j < 3; j++) {
            board[i][j] = 0;
        }
    }
}

void printBoard(int board[][3], int size) {
    cout << "-----" << endl;
    for (int i = 0; i < size; i++) {
        for (int j = 0; j < 3; j++) {
            cout << board[i][j] << " ";
        }
        cout << endl;
    }
    cout << "-----" << endl;
}

bool isBoardFilled(int board[][3]) {
    bool isFilled = true;
    for (int i = 0; i < 3; i++) {
        for (int j = 0; j < 3; j++) {
            if (board[i][j] == 0) isFilled = false;
        }
    }
    return isFilled;
}

int main() {

    int board[3][3];
    initBoard(board, 3);
    printBoard(board, 3);

    int playerOnesTurn = true;
    while (true) {
        int x, y;

        if (playerOnesTurn) {            
            cout << "Player 1's turn" << endl;
            do {
                cout << "Enter x coordinate: "; cin >> x;
                cout << "Enter y coordinate: "; cin >> y;
            } while (!fillPlace(board, x, y, 1));
        } else {
            cout << "Player 2's turn" << endl;
            do {
                cout << "Enter x coordinate: "; cin >> x;
                cout << "Enter y coordinate: "; cin >> y;
            } while (!fillPlace(board, x, y, 2));
        }
        playerOnesTurn = !playerOnesTurn;

        if (isBoardFilled(board)) {
            cout << "The game is a tie.\n";
            break;
        }

        printBoard(board, 3);
        if (checkWin(board, 1)) {
            cout << "Player 1 has won\n";
            break;
        } else if (checkWin(board, 2)) {
            cout << "Player 2 has won\n";
            break;
        }
    }

    return 0;
}
\$\endgroup\$
5
\$\begingroup\$

Avoid using namespace std

This can cause name collisions because it adds every name in the std namespace to the global namespace. For a small program like this one it's unlikely that you'll run into any problems (then again, maybe not) but it's best to get into the habit of using the std:: prefix on names in the std namespace.

Alternatively, you can introduce using declarations like using std::cout; to add specific names to the global namespace.

Avoid std::endl in favor of \n

std::endl flushes the stream, which can cause an unnecessary loss in performance. In printBoard() you use std::endl three times but you can easily replace some of those uses with \n.

Use a class for the Tic Tac Toe board

Instead of using a 3x3 array it's better to create a class to hold the state of the Tic Tac Toe board, with functions to set or get the value of a place on the board, determine whether or not the board is filled, etc. This allows you to hide implementation details which may change later on -- for example, if your compiler supports C++11 or will do so in the future you may want to change your internal representation of the board to use std::array. If you have a class for the Tic Tac Toe board the external interface would not change, only the code that implements the functionality.

Here's what that class might look like:

class Board {
    int board[3][3];
public:
    typedef int value_type;

    // More modern alternative to typedef:
    //using value_type = int;

    Board() {
        for (std::size_t i = 0; i < 3; i++) {
            for (std::size_t j = 0; j < 3; j++) {
                board[i][j] = 0;
            }
        }
    }

    // Gets the value of a board place
    int place(std::size_t x, std::size_t y) const {
        // Might want to check coordinates and throw an exception (e.g. std::out_of_range) if coordinate(s) are invalid
        return board[x][y];
    }

    // Sets the value of a board place
    // It would probably be better to throw an exception (e.g. std::out_of_range) instead of printing to std::cout
    bool place(std::size_t x, std::size_t y, int value) {
        if (x > 2 || x < 0) {
            cout << "Invalid x coordinate. Try again.\n";
            return false;
        } else if (y > 2 || y < 0) {
            cout << "Invalid y coordinate. Try again.\n";
            return false;
        } else if (board[x][y] != 0) {
            cout << "Place is already filled. Try again.\n";
            return false;
        } else {
            board[x][y] = value;
            return true;
        }
    }

    bool is_filled() const {
        for (int i = 0; i < 3; i++) {
            for (int j = 0; j < 3; j++) {
                // Return immediately once we find a non-filled place
                if (board[i][j] == 0) return false;
            }
        }

        // All places were found to be filled
        return true;
    }
};

(Note the use of std::size_t for the coordinate type and the typedef for the int value type, which mimic practices from the standard library.)

You would no longer need initBoard() since the Board constructor performs the initialization, and some of your other functions would become member functions of the class. In some cases you can improve the performance of these functions -- e.g. you can keep a count of how many places are filled by incrementing the count each time a new place is filled by Board::place(), and then Board::is_filled() simply has to check if that count equals 9 rather than checking every place in the board (the performance gain is negligible for such a tiny board but the principle is important).

Overload operator<< for printing the board

printBoard() only prints to std::cout. You can overload operator<< in order to insert the board into any std::ostream. Using the above Board class the code would look like this:

std::ostream& operator<<(std::ostream& os, Board board) {
    os << "-----\n";

    for (int i = 0; i < 3; i++) {
        for (int j = 0; j < 3; j++) {
            os << board.place(i, j) << " ";
        }

        os << '\n';
    }

    os << "-----" << std::endl;

    return os;
}

(Note the replacement of some of the std::endl uses with '\n`.)

To print the board to std::cout you'd simply use std::cout << board;, and you can use the same function to print the board to a different std::ostream such as a std::fstream (for writing to a file).

Make your code DRYer

In some cases you can make your code DRYer. For example, the code in the if and else sections is very similar here:

if (playerOnesTurn) {            
    cout << "Player 1's turn" << endl;
    do {
        cout << "Enter x coordinate: "; cin >> x;
        cout << "Enter y coordinate: "; cin >> y;
    } while (!fillPlace(board, x, y, 1));
 } else {
    cout << "Player 2's turn" << endl;
    do {
        cout << "Enter x coordinate: "; cin >> x;
        cout << "Enter y coordinate: "; cin >> y;
    } while (!fillPlace(board, x, y, 2));
 }

You can put that in a function like this:

// Capture Board by reference so we modify the original Board, not a copy
void player_move(Board& board, int player) {
    cout << "Player " << player << "'s turn" << endl;

    std::size_t x, y;

    do {
        cout << "Enter x coordinate: "; cin >> x;
        cout << "Enter y coordinate: "; cin >> y;
    } while (!board.place(x, y, player));
}

Here's a demo program with some of the changes I suggested:

#include <iostream>
#include <ostream>

using namespace std;

class Board {
    int board[3][3];
public:
    typedef int value_type;

    // More modern alternative to typedef:
    //using value_type = int;

    Board() {
        for (std::size_t i = 0; i < 3; i++) {
            for (std::size_t j = 0; j < 3; j++) {
                board[i][j] = 0;
            }
        }
    }

    // Gets the value of a board place
    int place(std::size_t x, std::size_t y) const {
        // Might want to check coordinates and throw an exception (e.g. std::out_of_range) if coordinate(s) are invalid
        return board[x][y];
    }

    // Sets the value of a board place
    // It would probably be better to throw an exception (e.g. std::out_of_range) instead of printing to std::cout
    bool place(std::size_t x, std::size_t y, int value) {
        if (x > 2 || x < 0) {
            cout << "Invalid x coordinate. Try again.\n";
            return false;
        } else if (y > 2 || y < 0) {
            cout << "Invalid y coordinate. Try again.\n";
            return false;
        } else if (board[x][y] != 0) {
            cout << "Place is already filled. Try again.\n";
            return false;
        } else {
            board[x][y] = value;
            return true;
        }
    }

    bool is_filled() const {
        for (int i = 0; i < 3; i++) {
            for (int j = 0; j < 3; j++) {
                // Return immediately once we find a non-filled place
                if (board[i][j] == 0) return false;
            }
        }

        // All places were found to be filled
        return true;
    }
};

bool checkWin(Board& board, int player) {
    // horizontal
    for (int i = 0; i < 3; i++) {
        if (board.place(i, 0) == player && board.place(i, 1) == player && board.place(i, 2) == player) return true;
    }

    // vertical
    for (int i = 0; i < 3; i++) {
        if (board.place(0, i) == player && board.place(1, i) == player && board.place(2, i) == player) return true;
    }

    // diagonal
    if ((board.place(0, 0) == player && board.place(1, 1) == player && board.place(2, 2) == player) ||
        (board.place(0, 2) == player && board.place(1, 1) == player && board.place(2, 0) == player))
        return true;

    return false;
}

// TODO implement function if the game is a tie before the last move

std::ostream& operator<<(std::ostream& os, Board board) {
    os << "-----\n";

    for (int i = 0; i < 3; i++) {
        for (int j = 0; j < 3; j++) {
            os << board.place(i, j) << " ";
        }

        os << '\n';
    }

    os << "-----" << std::endl;

    return os;
}

// Capture Board by reference so we modify the original Board, not a copy
void player_move(Board& board, int player) {
    cout << "Player " << player << "'s turn" << endl;

    std::size_t x, y;

    do {
        cout << "Enter x coordinate: "; cin >> x;
        cout << "Enter y coordinate: "; cin >> y;
    } while (!board.place(x, y, player));
}

int main() {
    Board board;
    std::cout << board;

    // should be a bool, not an int
    // alternatively, just use an int which is 1 when it's player 1's turn and 2 when player 2's turn
    bool playerOnesTurn = true;

    while (true) {
        player_move(board, playerOnesTurn ? 1 : 2);

        playerOnesTurn = !playerOnesTurn;

        if (board.is_filled()) {
            cout << "The game is a tie.\n";
            break;
        }

        std::cout << board;

        if (checkWin(board, 1)) {
            cout << "Player 1 has won\n";
            break;
        } else if (checkWin(board, 2)) {
            cout << "Player 2 has won\n";
            break;
        }
    }

    return 0;
}

I left a few other minor tips in the comments and I didn't make all the changes I suggested (e.g. I did not remove using namespace std).

\$\endgroup\$
3
\$\begingroup\$

Bug: if the winning move is the one that fills the board, the game ends in draw instead of a win. This is because you check for a draw before you check for a win.

You can simplify things a bit: only the current player can win, so there is no need to check if the other player has won.

main doesn’t need a return 0, it is implied.

Besides the redundancies already pointed out, checkWin is also a bit redundant, you can simply check to see if three values in a row are equal, it doesn’t matter what value they have. Thus,

board[0,i] == board[1,i] && board[0,i] == board[2,i]

suffices.

That said, a lot of the work done here would be simpler if the board was stored in a flat array with index = i + 3*j.

In this case, resetting the board is a simple call to std::fill, and checking to see if the board is full is a single call to std::all_of. Of course, if the board is a class, as suggested in the earlier answer, then you can implement an indexing operation that converts the (i,j) pair to a linear index.

\$\endgroup\$
  • \$\begingroup\$ FWIW, I disagree with an implicit return 0 on main(). Syntax should not be special. Explicit return values are useful. \$\endgroup\$ – Reinderien Nov 29 '18 at 1:01

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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