1
\$\begingroup\$

I'm new to programming. I'm an aspiring computer science major and have started taking a C++ course at my college. I was bored tonight, so I decided to see if I could make a working Tic-Tac-Toe game. I'd thought about it before, but had no idea how. Tonight it hit me, so I wrote it up!

Being amateur, I'm sure there are countless improvements I can make to ensure reliability and improve the overall efficiency of the program. As of now, I feel like there's an excessive amount of Boolean variables, but I could be wrong.

Here is my code. I have libraries Windows.h and iostream included. It works as intended, I just feel like it could be improved.

#include <iostream>
#include <Windows.h>

using namespace std;

int main() {
         //Default values for the board.
    char square1 = ' ',
         square2 = ' ',
         square3 = ' ',
         square4 = ' ',
         square5 = ' ',
         square6 = ' ',
         square7 = ' ',
         square8 = ' ',
         square9 = ' ';

         //Slot checks.
    bool bsquare1 = false,
         bsquare2 = false,
         bsquare3 = false,
         bsquare4 = false,
         bsquare5 = false,
         bsquare6 = false,
         bsquare7 = false,
         bsquare8 = false,
         bsquare9 = false,

         //Player 1 checks.
         square11 = false,
         square12 = false,
         square13 = false,
         square14 = false,
         square15 = false,
         square16 = false,
         square17 = false,
         square18 = false,
         square19 = false,

         //Player 2 checks.
         square21 = false,
         square22 = false,
         square23 = false,
         square24 = false,
         square25 = false,
         square26 = false,
         square27 = false,
         square28 = false,
         square29 = false,

         //Player turn check.
         player1;

    //Choice for play again or end program.
    char yn;

    //Square selection choice.           
    int choice;

    //Prints the board with examples.
    cout << endl
         << "\t" << "1 | 2 | 3" << endl
         << "\t" << "---------" << endl
         << "\t" << "4 | 5 | 6" << endl
         << "\t" << "---------" << endl
         << "\t" << "7 | 8 | 9" << endl << endl;

    cout << "Take note of the numbers above. Each number relates to a position. When asked to choose a square, you'll enter" << endl
         << "the number of the square you want. Press enter to begin! ";
    cin.get();
    system("CLS");

    cout << endl;

    //Blank board.  
    cout << "\t" << "  |   |  " << endl
         << "\t" << "---------" << endl
         << "\t" << "  |   |  " << endl
         << "\t" << "---------" << endl
         << "\t" << "  |   |  " << endl << endl;

    //Gets a value for player.
    for (int player = 1; player < 10; player++) {

        //If player value isn't even, it's player 1's turn. If it is even, it isn't player 1's turn, so it must be player 2's.
        if ((player % 2) == 1)
            player1 = true;
        else
            player1 = false;

        //If player1 is true, ask which square they wish to choose. 
        if (player1) {

            cout << "It's player 1's turn. Please choose a square. ";
            cin  >> choice;

            //If anything that isn't 1-9 is chosen, print an error. If a square is occupied already, print an error. Prompt to try again.
            while (choice < 0 || choice > 10 || (choice == 1 && bsquare1) || (choice == 2 && bsquare2) || (choice == 3 && bsquare3) || (choice == 4 && bsquare4) || (choice == 5 && bsquare5) || (choice == 6 && bsquare6) || (choice == 7 && bsquare7) || (choice == 8 && bsquare8) || (choice == 9 && bsquare9)) {

                cout << "Invalid option. Try again: ";
                cin  >> choice;

            }

            system("CLS");

            //Updates square depending on player choice. bsquare(n) is marked true so it can't be selected again.
            switch (choice) {
                case 1: 
                    square1 = 'X';
                    bsquare1 = true;
                    square11 = true;
                    break;
                case 2: 
                    square2 = 'X';
                    bsquare2 = true;
                    square12 = true;
                    break;
                case 3: 
                    square3 = 'X';
                    bsquare3 = true;
                    square13 = true;
                    break;
                case 4: 
                    square4 = 'X';
                    bsquare4 = true;
                    square14 = true;
                    break;
                case 5: 
                    square5 = 'X';
                    bsquare5 = true;
                    square15 = true;
                    break;
                case 6: 
                    square6 = 'X';
                    bsquare6 = true;
                    square16 = true;
                    break;
                case 7: 
                    square7 = 'X';
                    bsquare7 = true;
                    square17 = true;
                    break;
                case 8: 
                    square8 = 'X';
                    bsquare8 = true;
                    square18 = true;
                    break;
                case 9: 
                    square9 = 'X';
                    bsquare9 = true;
                    square19 = true;
                    break;
            }
        }

        //If not player 1, then player 2.
        if (!player1) {

            cout << "It's player 2's turn. Please choose a square. ";
            cin  >> choice;

            while (choice < 0 || choice > 10 || (choice == 1 && bsquare1) || (choice == 2 && bsquare2) || (choice == 3 && bsquare3) || (choice == 4 && bsquare4) || (choice == 5 && bsquare5) || (choice == 6 && bsquare6) || (choice == 7 && bsquare7) || (choice == 8 && bsquare8) || (choice == 9 && bsquare9)) {

                cout << "Invalid option. Try again: ";
                cin  >> choice;

            }

            system("CLS");

            switch (choice) {
                case 1: 
                    square1 = 'O';
                    bsquare1 = true;
                    square21 = true;
                    break;
                case 2: 
                    square2 = 'O';
                    bsquare2 = true;
                    square22 = true;
                    break;
                case 3: 
                    square3 = 'O';
                    bsquare3 = true;
                    square23 = true;
                    break;
                case 4: 
                    square4 = 'O';
                    bsquare4 = true;
                    square24 = true;
                    break;
                case 5: 
                    square5 = 'O';
                    bsquare5 = true;
                    square25 = true;
                    break;
                case 6: 
                    square6 = 'O';
                    bsquare6 = true;
                    square26 = true;
                    break;
                case 7: 
                    square7 = 'O';
                    bsquare7 = true;
                    square27 = true;
                    break;
                case 8: 
                    square8 = 'O';
                    bsquare8 = true;
                    square28 = true;
                    break;
                case 9: 
                    square9 = 'O';
                    bsquare9 = true;
                    square29 = true;
                    break;
            }

        }

        cout << endl;

        //Prints out board with updated values.
        cout << "\t" << square1 << " | " << square2  << " | " << square3 << endl
             << "\t" << "---------" << endl
             << "\t" << square4 << " | " << square5  << " | " << square6 << endl
             << "\t" << "---------" << endl
             << "\t" << square7 << " | " << square8  << " | " << square9 << endl << endl;

        //All possibilities for player 1 to win.
        if ((square11 && square12 && square13) || (square14 && square15 && square16) || (square17 && square18 && square19) ||
            (square11 && square14 && square17) || (square12 && square15 && square18) || (square13 && square16 && square19) ||
            (square11 && square15 && square19) || (square17 && square15 && square13)) {

            cout << "Player one wins!" << endl
                 << "Would you like to play again? (y/n) ";
            cin  >> yn;

            while (!(yn == 'Y' || yn == 'N' || yn == 'y' || yn == 'n')) {
                cout << "Choose either \"y\" or \"n\" and try again! ";
                cin  >> yn;
            }

            //If yes is selected, call function main, which resets all variables and starts the program over.
            if (yn == 'Y' || yn == 'y') {
                cout << "Get ready!" << endl;
                Sleep(2000);
                system("CLS");
                main();
            //If no is selected, return and exit the program.               
            } else {
                cout << "Thanks for playing!";
                Sleep(2000);
                return(0);
            }

        //All possibilities for player 2 to win.
        } else if ((square21 && square22 && square23) || (square24 && square25 && square26) || (square27 && square28 && square29) ||
                   (square21 && square24 && square27) || (square22 && square25 && square28) || (square23 && square26 && square29) ||
                   (square21 && square25 && square29) || (square27 && square25 && square23)) {

            cout << "Player two wins!" << endl
                 << "Would you like to play again? (y/n) ";
            cin  >> yn;

            while (!(yn == 'Y' || yn == 'N' || yn == 'y' || yn == 'n')) {
                cout << "Choose either \"y\" or \"n\" and try again! ";
                cin  >> yn;
            }

            if (yn == 'Y' || yn == 'y') {
                cout << "Get ready!" << endl;
                Sleep(2000);
                system("CLS");
                main();
            } else {
                cout << "Thanks for playing!";
                Sleep(2000);
                return(0);
            }

        //If all squares have been set to true and no one has won yet, the game is a draw.
        } else if (bsquare1 && bsquare2 && bsquare3 && bsquare4 && bsquare4 && bsquare6 && bsquare7 && bsquare8 && bsquare9) {

            cout << "It's a draw!" << endl
                 << "Would you like to play again? (y/n) ";
            cin  >> yn;

            while (!(yn == 'Y' || yn == 'N' || yn == 'y' || yn == 'n')) {
                cout << "Choose either \"y\" or \"n\" and try again! ";
                cin  >> yn;
            }

            if (yn == 'Y' || yn == 'y') {
                cout << "Get ready!" << endl;
                Sleep(2000);
                system("CLS");
                main();
            } else {
                cout << "Thanks for playing!";
                Sleep(2000);
                return(0);
            }
        }
    }
    return 0;
}
\$\endgroup\$
1
\$\begingroup\$
  1. Why did you not use arrays? This is one of those situations which naturally call for the use of arrays. In fact, you could've reduced the size of your while condition significantly. Instead of choice == x && bsquarex, you could've done it in a single condition

    if(choice<0 || choice >10 || bsquare[choice])

Another place where this would've been of use is in your win condition check. you've done all the checks hardcoded although you could've significantly compressed that by using arrays, and analysing win patterns; for example, one win pattern could be same row and another could be same column. avoid hardcoding long conditions as much as possible. hint: if your check condition rolls off your screen, think about how you could shorten it.

\$\endgroup\$
  • \$\begingroup\$ Oh wow! I knew there had to be some way to shorten those, I just didn’t know how. I’ll read up on arrays tonight and apply them to this code! Thanks for the info! \$\endgroup\$ – invadingdingo Oct 27 '17 at 13:17
1
\$\begingroup\$

I would suggest replacing the use of magic numbers with const or cpp #defines which are informative and descriptive, e.g.

 for (int player = 1; player < 10; player++) {

should be:

  for (int player = 1; player < NUM_OF_PLAYS; player++) {

or something similar.

Explicitly wrapping conditionals with brackets where required is also good practice.

 while (choice < 0 || choice > 10 ....

Should be more like:

while ((choice < 0) || (choice > 10) ....

and some developers like to go even further and separate the brackets as such:

while ( (choice < 0) || (choice > 10) ....  

The recursive function calls to main() are bad practice. I'm assuming this is due to your desire to have the user replay the game? Instead you should encapsulate your functionality and re-execute the function, rather than recursively calling main(). This would likely result in stack overflow occurring.

            system("CLS");
            main();
\$\endgroup\$
  • \$\begingroup\$ I'll definitely do this. Should I instead use a do/while loop that exits if the user selects no for play again? I feel like that might be better. It could eliminate the 3 return 0; 's that I have and use only the one at the end of the program. \$\endgroup\$ – invadingdingo Oct 27 '17 at 16:12
  • \$\begingroup\$ I would break up the problem into smaller problems such as: abstract functionality to a single or multiple methods; call method from main loop (handle single games only); implement the replay game functionality. That may make it clearer on how to handle the selection/monitoring of playing the game again. Also - multiple returns are bad practice. \$\endgroup\$ – S. Whittaker Oct 30 '17 at 7:45
0
\$\begingroup\$

You number your squares 1..9 but you check

choice < 0 || choice > 10

So if i have chose 0 or 10 the validation will fail, should have been

choice < 1 || choice > 9

You use the variable name player where the concept should have been turn or move.

Changing the board to an array would nearly reduce your program to 1/9 of the current code.

Having the same code duplicated for player1 and player2 is also not good, use the DRY principle (Don't Repeat Yourself) and use a function with player as parameter. In this case it could nearly half the code again.

\$\endgroup\$
  • \$\begingroup\$ I ended up fixing the choice validations when I tried entering ten and the program messed up, haha! No idea why I didn't catch that when I was writing it.. Changing the player variable sounds like a good idea. I'm having a little trouble understanding arrays. We haven't gone over it in class yet, so I am trying to teach myself. I get that it's a variable capable of holding different values, but I don't know how I'd apply it to this program.. \$\endgroup\$ – invadingdingo Oct 29 '17 at 18:45

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.