C++

Forum

 
Please review my code.

Momoironeko (13)
Hello, so I've being learning C++ on my free time for a few months now and I was wondering how I could make this code better or more elegant. Two scores from 1 to 10 would also be appreciated one for functionality and another for how easy it is to follow. Thank you all who reply ^^.

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
 
    /******************************************************
     *             A Simple Tic-Tac-Toe Game.             *
     ******************************************************/

#include <iostream>
#include <cstdio>
#include <cstdlib>
using namespace std;

void reDraw();
bool bNotGameOver();
bool bCheckValid(int, int, int);

char g_charBoard[9] = {'1', '2', '3', '4', '5', '6', '7', '8', '9'};
bool g_bIsDraw = 0;

int main()
{

    /******************************************************
     *             Welcomes the player.                   *
     ******************************************************/

    cout << "Would you Like to play Tic-Tac-Toe?" << endl;
    cout << "Yes or no?" << endl;
    string strChoice = "abc";

    while (strChoice == "abc")
    {
        cin >> strChoice;
        if (strChoice == "yes")
            break;
        else if (strChoice == "no")
        {
            cout << endl << "That's too bad." << endl;
            return 0;
        }
        else
        {
            cout << endl << "Please enter a valid reply." << endl;
            strChoice = "abc";
        }
    }

    /******************************************************
     *  Main game loop bNotGameOver checks for gameover.  *
     ******************************************************/

    int nPlayer = 1; // Makes it so player 1 goes first.
    reDraw();

    do
    {
        cout << "Where would you like to place a mark Player "
            << nPlayer << "?" << endl;
        int nLocation;
        cin >> nLocation;

        if (bCheckValid(1, 9, nLocation))
        {
            if (nPlayer == 1)
            {
                g_charBoard[nLocation - 1] = 'O';
                nPlayer = 2;
            }
            else
            {
                g_charBoard[nLocation - 1] = 'X';
                nPlayer = 1;
            }
        }
        else
        {
            cout << endl << "Sorry you did not enter a valid response." << endl;
            system("PAUSE");
        }
        reDraw();
    } while (bNotGameOver());

    if (g_bIsDraw == true)
        cout << endl << "It's a draw.";
    else if (nPlayer == 2)
        cout << endl << "Congratz Player 1!" << endl;
    else if (nPlayer == 1)
        cout << endl << "Congratz Player 2!" << endl;
    system("PAUSE");
    return 0;
}

    /******************************************************
     *Redraws the game board and clears the previous board*
     ******************************************************/

void reDraw()
{
    system("cls");
    cout << endl;
    cout << "\t" << g_charBoard[0] << "\t" << g_charBoard[1] << "\t" << g_charBoard[2] << endl << endl << endl;
    cout << "\t" << g_charBoard[3] << "\t" << g_charBoard[4] << "\t" << g_charBoard[5] << endl << endl << endl;
    cout << "\t" << g_charBoard[6] << "\t" << g_charBoard[7] << "\t" << g_charBoard[8] << endl << endl << endl;
}

bool bNotGameOver()
{
    if (g_charBoard[0] == g_charBoard[1] && g_charBoard[1] == g_charBoard[2])
        return false;
    else if (g_charBoard[3] == g_charBoard[4] && g_charBoard[4] == g_charBoard[5])
        return false;
    else if (g_charBoard[6] == g_charBoard[7] && g_charBoard[7] == g_charBoard[8])
        return false;
    else if (g_charBoard[0] == g_charBoard[4] && g_charBoard[4] == g_charBoard[8])
        return false;
    else if (g_charBoard[2] == g_charBoard[4] && g_charBoard[4] == g_charBoard[6])
        return false;
    else if (g_charBoard[0] == g_charBoard[3] && g_charBoard[3] == g_charBoard[6])
        return false;
    else if (g_charBoard[1] == g_charBoard[4] && g_charBoard[4] == g_charBoard[7])
        return false;
    else if (g_charBoard[2] == g_charBoard[5] && g_charBoard[5] == g_charBoard[8])
        return false;
    else if ((g_charBoard[0] != '1') && (g_charBoard[1] != '2')&& (g_charBoard[2] != '3')
            && (g_charBoard[3] != '4') && (g_charBoard[4] != '5') && (g_charBoard[5] != '6')
             && (g_charBoard[6] != '7') && (g_charBoard[7] != '8') && (g_charBoard[8] != '9'))
            {
                g_bIsDraw = true;
                return false;
            }
    else
        return true;
}

    /******************************************************
     *     Checks if the reply was a valid one or not.    *
     ******************************************************/

bool bCheckValid(int nMin, int nMax, int nEntered)
{
    if(nEntered > nMax)
        return false;
    else if (nEntered < nMin)
        return false;
    else if ( g_charBoard[nEntered - 1] == 'O' )
        return false;
    else if ( g_charBoard[nEntered - 1] == 'X' )
        return false;
    else
        return true;
}
Last edited on
Paoletti301 (315)
This will be coming from someone who is also learning but there are some things i've noticed which i would change. obviously if someone more skilled coments take what that say over what i do.

for the while ( string = "abc" )
it could just be an infinite loop ex. while ( 1 ). the conditions that matter break the loop and you dont have to worry about add "string = abc" at the end.

in bCheckValid, i would either make constant variables in the main area or just hardcode them into the function, that way there can only be one location if you ever need to change them.

in bGameNotOver you could make it easier on yourself and use a for loop
ex for columns
1
2
3
4
 
for ( int i = 0 ; i < 3 ; i++ )
{
if ( array[ i ] == array[ i + 3 ] && array[ i ] == array[ i + 6 ] )
      return false;


bIsDraw doesnt look like its in the scope of bGameNotOverEDIT: at first glance i didnt realize it was a global variable

one that im not quite sure about so i wouldnt listen too it unless someone else agrees or provides more insight, but i think you could remove exit( EXIT_SUCCESS ) and just put return 0; there and you wouldnt need the windows include( EDIT: does work, dont know if it will really change much besides a bit less baggage )

but as i said, just some thoughts from a fellow student :D

EDIT:i just wanted to add, "\n" in strings will also do a new line, just so you dont have to do as many endl;

and after playing it just a little suggestion, reverse the numbers on the board that way the bottom three are 1 2 3, that way it lines up with your numpad.

besides that, great job :D
Last edited on
Momoironeko (13)
Thanks Paoletti,

I made the loop that way because
1
2
3
 
while (1)
     if (condition)
          break;


would just exit the if loop but run again on the while loop. I think unless I did something wrong when I tried that.

bIsDraw is a global variable I forgot the g_ because I was debugging and forgot to change it back. Fixed it now though.

The game works I played a few games by myself (forever alone) to make sure it was functional.

I derped and forgot that return ended functions and main() is a function lol
Paoletti301 (315)
if statements aren't loops so no it wont break out of it, i just tried it and indeed it does work :D

i tried it as well, good job on it. as forever having no one to play it with, welcome to the world of tic tac toe. try making an AI for it though :D
Momoironeko (13)
Thanks, I'll go add a vs comp mode and make bNotGameOver using for loops tomorrow and post my end result. I think I can mark this as solved now.
Momoironeko (13)
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163
164
165
166
 
    /******************************************************
     *             A Simple Tic-Tac-Toe Game.             *
     ******************************************************/

#include <iostream>
#include <cstdio>
#include <cstdlib>
using namespace std;

int aiChoice();
void reDraw();
bool bNotGameOver();
bool bCheckValid(int, int, int);

char g_charBoard[9] = {'1', '2', '3', '4', '5', '6', '7', '8', '9'};
bool g_bIsDraw = 0;
bool g_VsComp = 0;

int main()
{

    /******************************************************
     *             Welcomes the player.                   *
     ******************************************************/

    cout << "Would you Like to play Tic-Tac-Toe?" << endl;
    cout << "Yes or no?" << endl;
    string strChoice = "abc";

    while (1)
    {
        cin >> strChoice;
        if (strChoice == "yes")
            break;
        else if (strChoice == "no")
        {
            cout << endl << "That's too bad." << endl;
            return 0;
        }
        else
            cout << endl << "Please enter a valid reply." << endl;
    }

    cout << endl << "Would you Like to play vs an AI?" << endl;
    cout << "Yes or no?" << endl;

        while (1)
    {
        cin >> strChoice;
        if (strChoice == "yes")
        {
            g_VsComp = true;
            break;
        }
        else if (strChoice == "no")
            break;
        else
            cout << endl << "Please enter a valid reply." << endl;
    }

    /******************************************************
     *  Main game loop bNotGameOver checks for gameover.  *
     ******************************************************/

    int nPlayer = 1; // Makes it so player 1 goes first.
    reDraw();

    do
    {
        cout << "Where would you like to place a mark Player "
            << nPlayer << "?" << endl;
        int nLocation;

            if (nPlayer == 2 && g_VsComp)
                nLocation = aiChoice();
            else
                cin >> nLocation;

        if (bCheckValid(1, 9, nLocation))
        {
            if (nPlayer == 1)
            {
                g_charBoard[nLocation - 1] = 'O';
                nPlayer = 2;
            }
            else
            {
                g_charBoard[nLocation - 1] = 'X';
                nPlayer = 1;
            }
        }
        else if (nPlayer == 2 && g_VsComp)
            cout << endl;
        else
        {
            cout << endl << "Sorry you did not enter a valid response." << endl;
            system("PAUSE");
        }
        reDraw();
    } while (bNotGameOver());

    if (g_bIsDraw == true)
        cout << endl << "It's a draw." << endl;
    else if (nPlayer == 2)
        cout << endl << "Congratz Player 1!" << endl;
    else if (nPlayer == 1)
        cout << endl << "Congratz Player 2!" << endl;
    system("PAUSE");
    return 0;
}
     
int aiChoice()
{
    return rand() % 9 + 1;
}
    /******************************************************
     *Redraws the game board and clears the previous board*
     ******************************************************/

void reDraw()
{
    system("cls");
    cout << endl;
    for (int iii = 0; iii < 3 ; iii++)
    cout << "\t" << g_charBoard[0 + (iii * 3)] << "\t" << g_charBoard[1 + (iii * 3)] << "\t" << g_charBoard[2 + (iii * 3)] << endl << endl << endl;
}

bool bNotGameOver()
{
    for (int iii = 0; iii < 3 ; iii++)
    if (g_charBoard[0 + (iii * 3)] == g_charBoard[1 + (iii * 3)] && g_charBoard[1 + (iii * 3)] == g_charBoard[2 + (iii * 3)])
        return false;
    else if (g_charBoard[0 + iii] == g_charBoard[3 + iii] && g_charBoard[3 + iii] == g_charBoard[6 + iii])
        return false;
    else if (g_charBoard[0] == g_charBoard[4] && g_charBoard[4] == g_charBoard[8])
        return false;
    else if (g_charBoard[2] == g_charBoard[4] && g_charBoard[4] == g_charBoard[6])
        return false;
    else if ((g_charBoard[0] != '1') && (g_charBoard[1] != '2')&& (g_charBoard[2] != '3')
            && (g_charBoard[3] != '4') && (g_charBoard[4] != '5') && (g_charBoard[5] != '6')
             && (g_charBoard[6] != '7') && (g_charBoard[7] != '8') && (g_charBoard[8] != '9'))
            {
                g_bIsDraw = true;
                return false;
            }
    else
        return true;
}

    /******************************************************
     *     Checks if the reply was a valid one or not.    *
     ******************************************************/

bool bCheckValid(int nMin, int nMax, int nEntered)
{
    if(nEntered > nMax)
        return false;
    else if (nEntered < nMin)
        return false;
    else if ( g_charBoard[nEntered - 1] == 'O' )
        return false;
    else if ( g_charBoard[nEntered - 1] == 'X' )
        return false;
    else
        return true;
}
Topic archived. No new replies allowed.