C++

Forum

 
My TIC TAC TOE game

hacker804 (24)
Hi,I am a beginner in C++ and I have made the following Tic Tac Toe Game.Please comment on it and tell me how i can improve it.Thanks.


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
167
168
169
170
171
172
173
174
175
176
177
178
179
180
181
182
183
184
185
186
187
188
189
190
191
192
193
194
195
196
197
198
199
200
201
202
203
204
205
206
207
208
209
210
211
212
213
214
215
216
217
218
219
220
221
222
223
224
225
226
227
228
 
#include <iostream>
#include <cstdlib>
#include <cmath>
#include <ctime>
using namespace std;
int inputMove(char b[],int size,int move,bool p);
int checkMove(char a[],int size,bool p);
int checkMarking(bool mk[],int size);
int isMarked(bool m[]);
int main()
{
    bool x;
    bool player = true;
    bool result=1;
    const int size = 9;
    char board[size] = {'1','2','3','4','5','6','7','8','9'};
    bool marked[size];
    int move;
    while(result)
    {

        cout << board[0] << "|" << board[1] << "|" << board[2] << "\n-----" << endl;
        cout << board[3] << "|" << board[4] << "|" << board[5] << "\n-----" << endl;
        cout << board[6] << "|" << board[7] << "|" << board[8] << "\n-----" << endl;
        cout << "Enter number:";
        cin >> move;

        inputMove(board,size,move,player);
        player=!player;

        result = checkMove(board,size,player);

    }

}

int inputMove(char b[],int size,int move,bool p)
{

    if(p==true)
    {
        switch(move)
        {
        case 1:
            system("CLS");
            b[0]='X';
            return *b;
        case 2:
            system("CLS");
            b[1]='X';
            return *b;
        case 3:
            system("CLS");
            b[2]='X';
            return *b;
        case 4:
            system("CLS");
            b[3]='X';
            return *b;
        case 5:
            system("CLS");
            b[4]='X';
            return *b;
        case 6:
            system("CLS");
            b[5]='X';
            return *b;
        case 7:
            system("CLS");
            b[6]='X';
            return *b;
        case 8:
            system("CLS");
            b[7]='X';
            return *b;
        case 9:
            system("CLS");
            b[8]='X';
            return *b;
        default:
            cout << "INVALID MOVE!" << endl;
            cin.get();
            cin.get();
            system("CLS");
            return 0;
        }

    }
    else
    {

        switch(move)
        {
        case 1:
            system("CLS");
            b[0]='O';
            return *b;
        case 2:
            system("CLS");
            b[1]='O';
            return *b;
        case 3:
            system("CLS");
            b[2]='O';
            return *b;
        case 4:
            system("CLS");
            b[3]='O';
            return *b;
        case 5:
            system("CLS");
            b[4]='O';
            return *b;
        case 6:
            system("CLS");
            b[5]='O';
            return *b;
        case 7:
            system("CLS");
            b[6]='O';
            return *b;
        case 8:
            system("CLS");
            b[7]='O';
            return *b;
        case 9:
            system("CLS");
            b[8]='O';
            return *b;
        default:
            cout << "INVALID MOVE!" << endl;
            cin.get();
            cin.get();
            system("CLS");
            return 0;
        };

    }


};
int checkMove(char a[],int size,bool p)
{
    if(p==false)
    {

      if((a[0]=='X') && (a[1]=='X') && (a[2]=='X'))
    {
        cout << "PLAYER 1 WINS" << endl;
        return 0;
    }
    else if((a[2]=='X') && (a[5]=='X') && (a[8]=='X'))
    {
        cout << "PLAYER 1 WINS" << endl;
        return 0;
    }
    else if((a[3]=='X') && (a[4]=='X') && (a[5]=='X'))
    {
        cout << "PLAYER 1 WINS" << endl;
        return 0;
    }
    else if((a[6]=='X') && (a[7]=='X') && (a[8]=='X'))
    {
        cout << "PLAYER 1 WINS" << endl;
        return 0;
    }
    else if((a[0]=='X') && (a[3]=='X') && (a[6]=='X'))
    {
        cout << "PLAYER 1 WINS" << endl;
        return 0;
    }
    else if((a[1]=='X') && (a[4]=='X') && (a[7]=='X'))
    {
        cout << "PLAYER 1 WINS" << endl;
        return 0;
    }
    else if((a[0]=='X') && (a[4]=='X') && (a[8]=='X'))
    {
        cout << "PLAYER 1 WINS" << endl;
        return 0;
    }
    else if((a[2]=='X') && (a[4]=='X') && (a[6]=='X'))
    {
        cout << "PLAYER 1 WINS" << endl;
        return 0;
    };
    }else if((a[0]=='O') && (a[1]=='O') && (a[2]=='O'))
    {
        cout << "PLAYER 2 WINS" << endl;
        return 0;
    }
    else if((a[2]=='O') && (a[5]=='O') && (a[8]=='O'))
    {
        cout << "PLAYER 2 WINS" << endl;
        return 0;
    }
    else if((a[3]=='O') && (a[4]=='O') && (a[5]=='O'))
    {
        cout << "PLAYER 2 WINS" << endl;
        return 0;
    }
    else if((a[6]=='O') && (a[7]=='O') && (a[8]=='O'))
    {
        cout << "PLAYER 2 WINS" << endl;
        return 0;
    }
    else if((a[0]=='O') && (a[3]=='O') && (a[6]=='O'))
    {
        cout << "PLAYER 2 WINS" << endl;
        return 0;
    }
    else if((a[1]=='O') && (a[4]=='O') && (a[7]=='O'))
    {
        cout << "PLAYER 2 WINS" << endl;
        return 0;
    }
    else if((a[0]=='O') && (a[4]=='O') && (a[8]=='O'))
    {
        cout << "PLAYER 2 WINS" << endl;
        return 0;
    }
    else if((a[2]=='O') && (a[4]=='O') && (a[6]=='O'))
    {
        cout << "PLAYER 2 WINS" << endl;
        return 0;
    }

}


Last edited on
closed account (o3hC5Di1)
Hi there,

That's pretty good for a beginner I would say, well done!

Here's a few improvements you may want to consider:

Your checkMove() function can be shortened significantly by using another way to check for winning combinations, an example would be to use maths to calculate the field positions, an example:
Horizontal rows - check a field and that field +1 and +2
Vertical rows - check a field and that field +3 and +6
That's just one possibility however, you could also store the possible winning combinations in arrays and loop over those arrays to check if the fields are the same, for example:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
 
//note, i didn't really test this, might need some debugging
int winning_combinations = {0, 1, 2, 2, 5, 8}; //012 is one comb, 258 another, add others as desired
int player;
for (int i=0;  i < 6 ; i+=3)  //6 is the size of the array of winning combinations
{
    //example of following: if a[0] == a[1] AND a[0] == a[2] (first winning combination) then someone won
    if (a[winning_combination[i]] == a[winning_combinations[i+1]]  &&  a[winning_combinations[i]] == a[winning_combinations[i+2]])
    {
        //if the character is X, player 1, else player 2 - this is called a ternary conditional operator if you want to google it
        player = (a[winning_combination[i]] =='X') ? 1 : 2;
        std::cout << "PLAYER " << player << " WINS";
        break; //stop for loop
    }
}


On a sidenote, this function is not really returning anything.
Sure, you're making it return 0, but for no apparent reason, becuase it is always returning 0.
In these cases, where a function does something without requiring it to return a value use the void type, i.i void checkMove().

As for your inputMove(char b[],int size,int move,bool p) function, that can also be greatly shortened.
You don't need to make a case statement for every player and every board option, this can all be done as such:

1
2
3
4
 
cin >> move;  //now move is a number from 1-9 presumably

//if player is one, set field to X, otherwise 0, use move-1 because arrays start to count from 0
a[move-1] = player==true  ? 'X' : '0';


You will probably also want to build in some type of check if a field has not already been marked previously.

Let us know if you have any further questions.
I took the code a step further for you to learn from it, so hopefully with some help you'll understand what's going on there.

All the best,
NwN
Last edited on
hacker804 (24)
You will probably also want to build in some type of check if a field has not already been marked previously.


Yes I am still working on that.And Thanks for the help


Waiting for more suggestions.
hacker804 (24)
I shortened the program down.Here is the new version:

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
 
#include <iostream>
#include <cstdlib>
#include <cmath>
#include <ctime>
using namespace std;
int inputMove(char b[],int size,int move,bool p,bool mk[]);
int checkMove(char a[],int size,bool p);
int checkMarking(bool mk[],int size);
int isMarked(bool m[]);
int main()
{
    bool x;
    bool player = true;
    bool result=1;
    const int size = 9;
    char board[size] = {'1','2','3','4','5','6','7','8','9'};
    bool marked[size];
    int move;
    while(result)
    {
        cout << board[0] << "|" << board[1] << "|" << board[2] << "\n-----" << endl;       // Draw the Board
        cout << board[3] << "|" << board[4] << "|" << board[5] << "\n-----" << endl;
        cout << board[6] << "|" << board[7] << "|" << board[8] << "\n-----" << endl;
        cout << "Enter number:";
        cin >> move;    // Input Move
        inputMove(board,size,move,player,marked);// Draw the move on the board
//       checkMarking(marked,size);
        player=!player;                    // Change Player
        result = checkMove(board,size,player);  //Check for win or draw
    }
}

int inputMove(char b[],int size,int move,bool p,bool mk[])
{
    char temp;
    if(p==true)
    {
        temp='X';
    }
    else
    {
        temp='O';
    };
    switch(move)
    {
    case 1:
        system("CLS");
        b[0]=temp;
        return *b;
    case 2:
        system("CLS");
        b[1]=temp;
        return *b;
    case 3:
        system("CLS");
        b[2]=temp;
        return *b;
    case 4:
        system("CLS");
        b[3]=temp;
        return *b;
    case 5:
        system("CLS");
        b[4]=temp;
        return *b;
    case 6:
        system("CLS");
        b[5]=temp;
        return *b;
    case 7:
        system("CLS");
        b[6]=temp;
        return *b;
    case 8:
        system("CLS");
        b[7]=temp;
        return *b;
    case 9:
        system("CLS");
        b[8]=temp;
        return *b;
    default:
        cout << "INVALID MOVE!" << endl;
        cin.get();
        cin.get();
        system("CLS");
        return 0;
    }
};
int checkMove(char a[],int size,bool p)
{
    char temp;
    if(p==false)
    {
        temp='X';
    }
    else
    {
        temp='O';
    }
    if((a[0]==temp) && (a[1]==temp) && (a[2]==temp))
    {
        cout << "PLAYER 1 WINS" << endl;
        return 0;
    }
    else if((a[2]==temp) && (a[5]==temp) && (a[8]==temp))
    {
        cout << "PLAYER 1 WINS" << endl;
        return 0;
    }
    else if((a[3]==temp) && (a[4]==temp) && (a[5]==temp))
    {
        cout << "PLAYER 1 WINS" << endl;
        return 0;
    }
    else if((a[6]==temp) && (a[7]==temp) && (a[8]==temp))
    {
        cout << "PLAYER 1 WINS" << endl;
        return 0;
    }
    else if((a[0]==temp) && (a[3]==temp) && (a[6]==temp))
    {
        cout << "PLAYER 1 WINS" << endl;
        return 0;
    }
    else if((a[1]==temp) && (a[4]==temp) && (a[7]==temp))
    {
        cout << "PLAYER 1 WINS" << endl;
        return 0;
    }
    else if((a[0]==temp) && (a[4]==temp) && (a[8]==temp))
    {
        cout << "PLAYER 1 WINS" << endl;
        return 0;
    }
    else if((a[2]==temp) && (a[4]==temp) && (a[6]==temp))
    {
        cout << "PLAYER 1 WINS" << endl;
        return 0;
    };
}


And NwN,the function checkMove is returning 0 and that 0 is assigned to variable result due to which it becomes false and the loop terminates.
closed account (o3hC5Di1)
Hi there,

Sorry, I missed that bit on the return 0 stopping the loop.

You can still shorten by doing:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
 
int inputMove(char b[],int size,int move,bool p,bool mk[])
{
    char temp;
    if(p==true)
    {
        temp='X';
    }
    else
    {
        temp='O';
    };
    //check if valid move - this could also hold a check if the position is taken already
    if (move > 0 && move < 10) 
    {
        b[move-1] = temp;
    }
    else
    {
        cout << "invalid move";
    }
}


You'll have to adapt it to return values as you want it off course, but you get the idea.

Also, your checkMove function will now always say that player one has won even if player two wins.
Maybe you can set an extra variable besides temp that holds the player number and which can then be printed as cout << "player " << player_number << " wins";

Other than that, good stuff - you've already shortened your code with about 80 lines there!

All the best,
NwN
ResidentBiscuit (4459)
Why not just make checkMove return bool then?
Lowest0ne (1536)
inputMove can be shorter still
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
 
int inputMove(char b[],int size,int move,bool p)
{
//simple enough to ternary I would say
char temp = p ? 'X' : 'O';
if (size > 9)
{
  cout << "INVALID MOVE!" << endl;
  cin.get();
  cin.get();
  system("CLS");
  return 0;
}

system("CLS");
b[size-1]=temp;
return *b;
}


Writing it down, the return doesn't make any sense. You are returning the value of the first index of b. This should probably be a bool return also.

A little bug, if the move is invalid the player looses his/her move.

You also aren't ensuring the board position hasn't been used yet
Last edited on
hacker804 (24)
I have completed the tic tac toe program.
Comments are welcome so that i can improve my skills and make my next project even better.
HERE IT IS:
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
 
#include <iostream>
#include <cstdlib>
using namespace std;
int inputMove(char b[],int size,int move,bool p,bool mk[]);
bool checkMove(char a[],int size,bool p);
int main()
{
    int temp;
    bool player = true;
    bool result=1;
    const int size = 9;
    char board[size] = {'1','2','3','4','5','6','7','8','9'};
    bool marked[size] = {0,0,0,0,0,0,0,0,0};
    int move;
    while(result)
    {
        cout << board[0] << "|" << board[1] << "|" << board[2] << "\n-----" << endl;       // Draw the Board
        cout << board[3] << "|" << board[4] << "|" << board[5] << "\n-----" << endl;
        cout << board[6] << "|" << board[7] << "|" << board[8] << "\n-----" << endl;
        cout << "Enter number:";
        cin >> move;                                // Input Move
        temp=inputMove(board,size,move,player,marked);// Draw the move on the board
        if(temp==0)
        {
            player = player;
        }
        else
        {
            player=!player;
        }

        result = checkMove(board,size,player);  //Check for win or draw
    }
}

int inputMove(char b[],int size,int move,bool p,bool mk[])
{
    char temp;
    if(p==true)         //Determine who's move is it.
    {
        temp='X';
    }
    else
    {
        temp='O';
    };
    if(move>0 && move<10)       //Make the move and mark it.
    {
        system("CLS");
        if(mk[move-1]==1)
        {
            cout <<  "Already Marked" << endl;
            return 0;
        }
        b[move-1]=temp;
        mk[move-1]=1;

    }
    else                //If move is not between 1-9,ask the user to re-enter
    {
        cout << "INVALID MOVE" << endl;
        cin.get();
        cin.get();
        system("CLS");
    };
};
bool checkMove(char a[],int size,bool p)
{
    char temp;
    if(p==false)
    {
        temp='X';
    }
    else
    {
        temp='O';
    }
    if((a[0]==temp) && (a[1]==temp) && (a[2]==temp))
    {
        cout << temp << " WINS!" << endl;
        return 0;
    }
    else if((a[2]==temp) && (a[5]==temp) && (a[8]==temp))
    {
        cout << temp << " WINS!" << endl;
        return 0;
    }
    else if((a[3]==temp) && (a[4]==temp) && (a[5]==temp))
    {
        cout << temp << " WINS!" << endl;
        return 0;
    }
    else if((a[6]==temp) && (a[7]==temp) && (a[8]==temp))
    {
        cout << temp << " WINS!" << endl;
        return 0;
    }
    else if((a[0]==temp) && (a[3]==temp) && (a[6]==temp))
    {
        cout << temp << " WINS!" << endl;
        return 0;
    }
    else if((a[1]==temp) && (a[4]==temp) && (a[7]==temp))
    {
        cout << temp << " WINS!" << endl;
        return 0;
    }
    else if((a[0]==temp) && (a[4]==temp) && (a[8]==temp))
    {
        cout << temp << " WINS!" << endl;
        return 0;
    }
    else if((a[2]==temp) && (a[4]==temp) && (a[6]==temp))
    {
        cout << temp << " WINS!" << endl;
        return 0;
    }
    if((a[0] != '1') && (a[1] != '2') && (a[2] != '3') && (a[3] != '4')             //Check for Draw
            && (a[4] != '5') && (a[5] != '6') && (a[6] != '7') && (a[7] != '8')
            && (a[8] != '9'))
    {
        system("CLS");
        cout << "DRAW!";
        return 0;
    }
    return 1;
}


Also,thanks a lot for the help.
hacker804 (24)
Please i need your comments.

Thanks
Fransje (435)
I copied and pasted your revised code in my IDE, and when I ran it , I could only put X's in the board. Is that my fault, or a potential improvement for your code? :) And you could add some more features, like singleplayer.

EDIT: I tried it in Dev-C++
Last edited on
hacker804 (24)
I copied and pasted your revised code in my IDE, and when I ran it , I could only put X's in the board. Is that my fault, or a potential improvement for your code? :) And you could add some more features, like singleplayer.


Well it works fine in CodeBlocks.

Can someone else confirm this
closed account (o3hC5Di1)
Hi there,

I still stand by my point of reducing the size of your checkMove function as I mentioned here: http://cplusplus.com/forum/beginner/75571/#msg405456

Also this code:

1
2
3
4
5
6
7
8
 
if(p==false)
    {
        temp='X';
    }
    else
    {
        temp='O';
    }


Can be written as:

temp = (p==false) ? 'X' : 'O';

It's called the conditional operator and was specifically designed for this purpose.

All the best,
NwN
Last edited on
Topic archived. No new replies allowed.