2
\$\begingroup\$

We have been learning C++ the previous semester and I made this simple TicTacToe game at the end of it. We haven't yet covered topics of vectors, classes, objects, exception handling etc. so with this code I mostly wanted to practise use of arrays. I would like to hear from you on suggestions on how to make this code more readable, if there are any problems with it and, of course, possible improvements.

Since this game is played by the user as the player X and the computer as the player O, I am aware that the computer currently does not choose the best possible move, but rather randomly an empty field and that is something I plan to work on in the future.

Thank you in advance.

   #include <iostream>
   #include <ctime>
   #include <cstdlib>

    const int SIZE = 3;
    // tracks the status of all fields
    bool boardStatus[SIZE][SIZE];
    int score[2] = {};
    void printMenu();

    void drawBoard (bool m1[][SIZE], bool m2[][SIZE]){
       for (int i = 0; i < SIZE; i++){
           std::cout << "-------------\n" << "| ";
           for (int j = 0; j < SIZE; j++)
               if (m1[i][j]) std::cout << "X" << " | ";
               else if (m2[i][j]) std::cout << "O" << " | ";
               else std::cout << "  | ";
           std::cout << "\n";
       }
       std::cout << "-------------\n";
   }
   bool draw(){
       for (int i = 0; i < SIZE; i++)
           for (int j = 0; j < SIZE; j++)
                   if (!boardStatus[i][j]) return false;
       return true;
   }
   bool win(bool m[][SIZE]){
       int counter1 = 0, counter2 = 0;
       // checks row by row and column by column for
       // three X's or O's
       for (int i = 0; i < SIZE; i++){
           for (int j = 0; j < SIZE; j++){
               if (m[i][j]) counter1++;
               if (m[j][i]) counter2++;
           }
           if (counter1 == 3 || counter2 == 3) return true;
           counter1 = 0; counter2 = 0;
       }
       // checks left diagonal
       for (int i = 0; i < SIZE; i++)
           if (m[i][i]) counter1++;
       if (counter1 == 3) return true;
       // checks right diagonal
       for (int i = 0, j = 2; i < SIZE; i++, j--)
           if (m[i][j]) counter2++;
       if (counter2 == 3) return true;

       return false;
   }
   void printWinOrDraw (bool m[][SIZE], char ch){
       if (win(m)) {
           std::cout << ch << " player won!\n\n";
           if (ch == 'X') score[0]++;
           else score[1]++;
       }
       else if (draw()) std::cout << "It's a draw!\n\n";
       return;
   }
   // randomly generates field for player O
   void computerChoice(int &rowComputer, int &columnComputer){
       srand(time(0));
       do{
              rowComputer = rand() % 3;
              columnComputer = rand() % 3;
              if (!boardStatus[rowComputer][columnComputer])break;
           } while (boardStatus[rowComputer][columnComputer]);
   }
   // takes valid input for player X from the user
   void playerInput (int &rowPlayer, int &columnPlayer){
       do {
           do {
               std::cout << "Enter a row (0, 1 or 2) for player X: ";
               std::cin >> rowPlayer;
               if (rowPlayer < 0 || rowPlayer > 2)
               std::cout << "Invalid input! Try again\n";
           } while (rowPlayer < 0 || rowPlayer > 2);
           do {
               std::cout << "Enter a column (0, 1 or 2) for player X: ";
               std::cin >> columnPlayer;
               if (columnPlayer < 0 || columnPlayer > 2)
                   std::cout << "Invalid input! Try again\n";
           } while (columnPlayer < 0 || columnPlayer > 2);
           if (!boardStatus[rowPlayer][columnPlayer]) break;
           else std::cout << "That field is already taken. Use another field.\n";
       } while (boardStatus[rowPlayer][columnPlayer]);
   }
   void startGame(bool player[][SIZE], bool computer[][SIZE]){
       int rowPlayer, columnPlayer, rowComputer, columnComputer;
       do {
           playerInput(rowPlayer, columnPlayer);
           // marks player X field choice as taken
           boardStatus[rowPlayer][columnPlayer] = true;
           player[rowPlayer][columnPlayer] = true;
           drawBoard(player, computer);
           if (win(player) || draw()){
           printWinOrDraw(player, 'X');
           break;
           }
           computerChoice(rowComputer, columnComputer);
           std::cout << "O player chooses field (" << rowComputer << "," << columnComputer << "): \n";
           // marks player O field choice as taken
           boardStatus[rowComputer][columnComputer] = true;
           computer[rowComputer][columnComputer] = true;
           drawBoard(player, computer);
           if (win(computer) || draw()){
           printWinOrDraw(computer, 'O');
           break;
           }
       } while (!win(player) || !win(computer) || !draw());
       printMenu();
   }
   void setGame(){
       bool player[SIZE][SIZE], computer[SIZE][SIZE];
       // all fields on the board are initially marked
       // as empty (false)
       for (int i = 0; i < SIZE; i++)
           for (int j = 0; j < SIZE; j++) {
               boardStatus[i][j] = false;
               player[i][j] = false;
               computer[i][j] = false;
               }
       drawBoard(player, computer);
       startGame(player, computer);
   }
    void printMenu(){
        std::cout << "Choose one of following options by entering 1 or 2: "
            << "\n1. Reset the game "
            << "\n2. Exit\n";
       int option;
       std::cin >> option;
       switch(option){
           case 1: setGame(); break;
           case 2: std::cout << "\nSCORE:\nX | O\n" << score[0] << " : " << score[1] << "\n"; return;
           default: std::cout << "Invalid input. Game ends\n"; return;
       }
   }
   int main(){
       std::cout << "WELCOME TO THE GAME!\nYou are player X and the computer is player O.\nLet's start the game: \n";
       setGame();
       return 0;
   }  
\$\endgroup\$
3
\$\begingroup\$

Variable naming needs some work. SIZE, m, ch, counter1 etc. could use more thought out names indicating what they do or what they are related to.
Does win win the game? Apparently it checks for a win so a rename makes sense here.


int score[2] = {}; while allowed is redundant


Separate functions a bit. As it stands now it's somewhat hard to read so consider adding newlines between functions.


Look into random if you need randomization in your programs.


Mostly personal preference but adding braces where possible can save you from nasty bugs in the long run.


You have a variable for the board size yet the game breaks in various ways for sizes >3.

\$\endgroup\$
  • \$\begingroup\$ I should have put more thought to naming, I agree. Thank you for the suggestions. If you could clarify a bit more your last suggestion, that would be great. \$\endgroup\$ – J.J Mar 12 at 7:31
  • \$\begingroup\$ @J.J Try setting const int SIZE = 3; to 4, is the game still playable? \$\endgroup\$ – yuri Mar 12 at 17:31
1
\$\begingroup\$

When we include <ctime>, we get function names in the std namespace. The compiler is also allowed, but not required, to define the same names in the global namespace; we can't rely on those in a portable program.

So we need

std::srand(std::time(0));

A more serious issue is that the flow of control is all over the place. For example, printMenu() not only displays the menu (as its name would suggest), but it also performs the user's requested action. See if you can make it into a function that displays the menu and returns the user's response, so that it's a reusable service for the caller, rather than taking over the flow of control. The problem you have here is that everything is very tightly coupled, meaning that none of the code can be used in any other program.

To improve the structure, think how to write the program so that either player can be operated by a computer (either in this program, or perhaps via a network protocol) or by a human (by a text or graphical interface, or via the same network protocol). If we can separate the game play from the player interaction, then we have a less tightly coupled program. If you can see how to do that, it's a principle that can be applied more generally, and it will serve you well.

If you find yourself struggling to decouple the parts of the program that are too tightly coupled, then there's a wealth of reading out there, pitched at varying levels - perhaps start with SOLID principles and follow the links (both the links to the five principles and the ones in the "References" section).

\$\endgroup\$
  • \$\begingroup\$ Readers are invited to suggest other reading - in comments, please. \$\endgroup\$ – Toby Speight Mar 12 at 22:12

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.