Thread: My first big program: X/O game

I recently started learning C++ from the internet.
This is my first big program, it's a simple naughts and crosses game. There is a 2 player mode and a player vs computer mode.
Making this game was much harder than I expected, and I must admit I'm proud of myself.
I'd like to know what you think about it and where I could improve it. I would really appreciate feedback! (Sorry in advance for the messy code).
Since the code is admittedly long and hard to read, I understand if you do not wish to devote so much of your time to it.

Code:

#include <cstdlib>
#include <time.h>
#include <stdio.h>
#include <iostream>
using namespace std;


string places[3][3] =     {{"Empty 1   ","Empty 2   ","Empty 3   "},           /* Places[3][3] is the game's board. This is where the Xs and Os will go */
                           {"Empty 4   ","Empty 5   ","Empty 6   "},
                           {"Empty 7   ","Empty 8   ","Empty 9   "}};
int lastmove;                                                                  // When playing against the computer, lastmove is 1 after the player's move and 0 after the computer's move. It thus enables us to know after whose move was a win achieved. Hence: who won.
char compXO;                                                                   // Determines whether the computer will play as X or as O
int win(int lastmove, int mode, string places[3][3], char XO, char compXO);
char XorO();
void table();
void placingX(char XO);
void printplaces(string places[3][3]);
int  gamemode();
void computermove(string places[3][3], char compXO, int mode);
string whowon(string places[3][3]);




int main()


{
srand(time(NULL));                                                             // srand enables use of random number generation (necessary for the computer's move)
table();
int mode = gamemode();
char XO = XorO();
if (XO == 'X') compXO = 'O'; else compXO = 'X';                               // Computer always plays for the opposite team


for (int turn = 1; turn <=9; turn++)                                          // The game loop. It's up to 9 because after 9 loops the board will be full and the game concluded
{
printplaces(places);                                                          // Prints the board to screen
placingX(XO);                                                                 // Let's the player make a move
lastmove = 1;


if (win(lastmove, mode, places, XO, compXO) == 1) goto END;                                      // Checks if the player's move won the game
if (turn == 9) {printplaces(places); break;}                                                                 // If we are in the last turn and the player's move didn't win the game, it is a draw. (This line prevents the computer from trying to make a move after the board has been filled completely and a win was not achieved
if (mode == 0)
    {
        computermove(places, compXO, mode);                                                              // Lets the computer make a move
        turn++;                                                             
        lastmove = 0;
        if (win(lastmove, mode, places, XO, compXO) == 1) goto END;                              //  Checks if the computer's move won the game
    }
if (mode == 1)                                                        // If we are in 2 player mode: After each turn the player will play as the opposite team
    {
        if (XO == 'X')   XO = 'O'; else if (XO == 'O')  XO = 'X';    // Alternates between X and O every turn
    }
}
cout << "\nGame Result: Draw!" << endl;                             // If the board has been filled completely without a win achieved, it is a draw.


END: system("pause");
return 0;
}




char XorO()          /* This function lets the player choose whether they will play as X or as O */
{
    char XO;
    int X0;


    choose:
    cout << "\n1 for X\n0 for O" << endl;
    scanf(" %d", &X0);
    if (X0 == 1)  XO = 'X'; else if (X0 == 0)   XO = 'O'; else {printf("\nYou did not follow the instruction\n"); goto choose;}
    return XO;
}


void table()         /* This function draws an example table which corresponds to where the user commands 1..9 will draw his X/O */
{
    int i,k;
    char table[3][3] = {{'1','2','3'},
                        {'4','5','6'},
                        {'7','8','9'}};
    cout << "Placing controls: " << endl;
    cout << "\n\n";
    for (i=0; i<3; i++)
    {
        for (k=0; k<3; k++)
        {
            cout << table[i][k] << "             ";
        }
      cout << "\n\n";
    }
    cout << "\n-----------------------------------------------------\n\n";
    return;
}




void placingX(char XO)          /* This function lets the user place his X/O in a chosen location in the "places" array (the game's board) and checks for the legality of his move */
{
    int place;


    again:
    printf("Where do you want to place your %C?\n", XO);
    cin >> place;


    switch(place)
    {
     case 1: if ((places[0][0] != "X         ") && (places[0][0] != "O         ")) {(places[0][0] = XO) += "         "; break;} else {cout << "\nThis spot is already taken!\n\n\n"; goto again;}
     case 2: if ((places[0][1] != "X         ") && (places[0][1] != "O         ")) {(places[0][1] = XO) += "         "; break;} else {cout << "\nThis spot is already taken!\n\n\n"; goto again;}
     case 3: if ((places[0][2] != "X         ") && (places[0][2] != "O         ")) {(places[0][2] = XO) += "         "; break;} else {cout << "\nThis spot is already taken!\n\n\n"; goto again;}
     case 4: if ((places[1][0] != "X         ") && (places[1][0] != "O         ")) {(places[1][0] = XO) += "         "; break;} else {cout << "\nThis spot is already taken!\n\n\n"; goto again;}
     case 5: if ((places[1][1] != "X         ") && (places[1][1] != "O         ")) {(places[1][1] = XO) += "         "; break;} else {cout << "\nThis spot is already taken!\n\n\n"; goto again;}
     case 6: if ((places[1][2] != "X         ") && (places[1][2] != "O         ")) {(places[1][2] = XO) += "         "; break;} else {cout << "\nThis spot is already taken!\n\n\n"; goto again;}
     case 7: if ((places[2][0] != "X         ") && (places[2][0] != "O         ")) {(places[2][0] = XO) += "         "; break;} else {cout << "\nThis spot is already taken!\n\n\n"; goto again;}
     case 8: if ((places[2][1] != "X         ") && (places[2][1] != "O         ")) {(places[2][1] = XO) += "         "; break;} else {cout << "\nThis spot is already taken!\n\n\n"; goto again;}
     case 9: if ((places[2][2] != "X         ") && (places[2][2] != "O         ")) {(places[2][2] = XO) += "         "; break;} else {cout << "\nThis spot is already taken!\n\n\n"; goto again;}
     default: goto again; break;
    }
    return;
}




void printplaces(string places[3][3])     /* This function re-draws the "places" array (the game's board) when necessary */
{
    int row,column;
    cout << "\n\nGame Board:\n\n";
    for (row=0; row<3; row++)
    {
        for (column=0; column<3; column++)
        {
            cout << places[row][column] << " ";
        }
      cout << "\n\n";
    }
    cout << "\n\n";
    return;
}


int  gamemode()                         /* This function lets the player select a game mode: Either Player vs Player or Player vs Computer */
{
    int mode;
    modeselect:
    cout << "\n1 for a two player game\n0 for a game against the computer\n";
    scanf(" %d", &mode);
    if (mode != 1 && mode != 0) {printf("\nYou did not follow the instruction\n"); goto modeselect;}
    return mode;
}


void computermove(string places[3][3], char compXO, int mode)             /* This function generates a random location on the board and then checks whether that spot is free. It keeps generating coordinates until a free spot is found, then places the X/O in that spot */
{
    if (mode == 1) return;
    random:
    int randomA = rand()%3;
    int randomB = rand()%3;
    if ((places[randomA][randomB] != "X         ") && (places[randomA][randomB] != "O         "))
    {
         (places[randomA][randomB] = compXO) += "         ";
    }
    else goto random;
    printplaces(places);
    cout << whowon(places);
    return;
}


string whowon(string places[3][3])                                      /* This function defines what a "win" is and checks if one has been achieved yet */
{
    string winstate;
    if ( ((places[0][0] == places[0][1]) && (places[0][2] == places[0][0])) || ((places[1][0] == places[1][1]) && (places[1][2] == places[1][0])) || ((places[2][0] == places[2][1]) && (places[2][2] == places[2][0])) || // Horizontal combo
         ((places[0][0] == places[1][0]) && (places[2][0] == places[0][0])) || ((places[0][1] == places[1][1]) && (places[2][1] == places[0][1])) || ((places[0][2] == places[1][2]) && (places[2][2] == places[0][2])) || // Vertical combo
         ((places[0][0] == places[1][1]) && (places[2][2] == places[0][0])) || ((places[0][2] == places[1][1]) && (places[2][0] == places[0][2])) ) // Diagonal combo


        {
        winstate = "Result: Win!";
        return winstate;
        }
        else
        {
        winstate = "\nStatus: No Winner Yet\n";
        return winstate;
        }
}


int win(int lastmove, int mode, string places[3][3], char XO, char compXO)                 /* This function will generate an output when a win is achieved */
{
        int win_achieved = 0;
        if (whowon(places) == "Result: Win!")
        {
                win_achieved = 1;
                printplaces(places);
                if (lastmove == 1 || mode == 1) {cout << "Game Result: " << XO << " WON!" << endl;}        // The win was achieved after a player's move. The player who made the last move is awarded the win


                else {cout << "Game Result: " << compXO << " WON!" << endl;}                              // A win was achieved, but it was not after a player's move, hence the computer must have scored the win
        }
        return win_achieved;
}

You really need to work on your formatting, including indentation. Avoid bunching multiple statements on a single line. Placing comments at the end of a statement works well when you have say, a declaration and wish to make a short elaboration about it, but when you are using comments to introduce a block of code, place them in a separate line before the code. You should also leave it out comments that pretty much say the same thing as the code, and sometimes the need to comment on a function call may indicate that your function is poorly named (e.g., "Prints the board to screen": if "printplaces" doesn't convey that to you, then rename printplaces, e.g., printBoardToScreen, after which you don't need the comment).

Here is how I might format your main function:

Code:

int main()
{
    srand(time(NULL));
    table();
    int mode = gamemode();
    char XO = XorO();
    // Computer always plays for the opposite team
    if (XO == 'X')
        compXO = 'O';
    else
        compXO = 'X';

    // The game loop. It's up to 9 because after 9 loops the board will be full and the game concluded
    for (int turn = 1; turn <= 9; turn++)
    {
        // Prints the board to screen
        printplaces(places);
        // Let's the player make a move
        placingX(XO);
        lastmove = 1;

        // Checks if the player's move won the game
        if (win(lastmove, mode, places, XO, compXO) == 1)
            goto END;
        // If we are in the last turn and the player's move didn't win the game, it is a draw.
        // (This prevents the computer from trying to make a move after the board has been filled completely and a win was not achieved)
        if (turn == 9)
        {
            printplaces(places);
            break;
        }

        if (mode == 0)
        {
            computermove(places, compXO, mode);
            turn++;
            lastmove = 0;
            //  Checks if the computer's move won the game
            if (win(lastmove, mode, places, XO, compXO) == 1)
                goto END;
        }
        // If we are in 2 player mode: After each turn the player will play as the opposite team
        if (mode == 1)
        {
            // Alternates between X and O every turn
            if (XO == 'X')
                XO = 'O';
            else if (XO == 'O')
                XO = 'X';
        }
    }
    // If the board has been filled completely without a win achieved, it is a draw.
    cout << "\nGame Result: Draw!" << endl;

END:
    system("pause");
    return 0;
}

I suspect that you are doing too much on each iteration: you iterate 9 times, ostensibly once for each tile on the board, so why does each iteration involve playing two moves, one per player, when mode == 0? I suspect this is why you need your turn == 9 check. With some restructuring (possibly pulling out the turn by turn gameplay code into another function), you may find that you also don't need the goto statements.

Speaking of goto statements: a glance at your computermove function shows that you used a goto to loop to some point above in the code. This means that you are overly reliant on goto. For practice, forbid yourself from using goto entirely. No goto. At all. So I'd change my mind on your main function and say that not only do you not need the goto statements, you need to get rid of them. Force yourself to use loop constructs. At some time you might become proficient enough and free from attachment to goto where say, you might legitimately break out of a nested loop with a goto, but for now, don't even do that.

Furthermore, you should use named constants, not magic numbers like 0 and 1 for the mode. mode == HUMAN_VS_COMPUTER (or mode == ONE_PLAYER) and mode == TWO_PLAYER would be easier to make sense of than mode == 0 and mode == 1.

Originally Posted by crazy_zimuki

May I ask though, why are global variables "bad"? To me they are the easiest to use and can be passed between functions with ease.

They are a simple and lazy way to do things that in large programs tend to result in hard to test code.

Once you learn to use functions and pointers you will rarely find a true need for global variables; the one common exception that I find is that in embedded programming they are often needed to handle interrupts and on some embedded systems needed for other reasons.

Tim S.