Looking for friendly advice

Dec 30, 2012 at 7:48am
Hi, I have just written my first Tic Tac Toe game. I was just looking for some input on how to make it a little more compact/easy to read. Also looking possibly an easier way to switch between first and second player that may lead to an easier way to check winning. Thank-You in advanced.

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
#include <iostream>


using namespace std;
void board_draw();
int selection;
char board[10] = {'0','1','2','3','4','5','6','7','8','9'};


int main()
{


    int player_move;
    int player1 = -1;
    int player2 = 0;
    //int selction;
while(player2<9){
    board_draw();//Draw board by initilizing function
    //cout<<"Please make a move by selcting your desired space";
    //cin>>selection;


    if(player1<player2){//Check turn
            cout<<"Player 1, Please make a move by selcting your desired space";
            cin>>selection;
        switch(selection){
    case 1:
        board[1] = 'X';
        break;
    case 2:
        board[2] = 'X';
        break;
    case 3:
        board[3] = 'X';
        break;
    case 4:
        board[4] = 'X';
        break;
    case 5:
        board[5] = 'X';
        break;
    case 6:
        board[6] = 'X';
        break;
    case 7:
        board[7] = 'X';
        break;
    case 8:
        board[8] = 'X';
        break;
    case 9:
        board[9] = 'X';
        break;
    default:
            cout<<"Please make a valid selection.";
        }
            player1++;
    }
    else if(player2 == player1){
        cout<<"Player 2, Please make a move by selcting your desired space";
            cin>>selection;
        switch(selection){
    case 1:
        board[1] = 'O';
        break;
    case 2:
        board[2] = 'O';
        break;
    case 3:
        board[3] = 'O';
        break;
    case 4:
        board[4] = 'O';
        break;
    case 5:
        board[5] = 'O';
        break;
    case 6:
        board[6] = 'O';
        break;
    case 7:
        board[7] = 'O';
        break;
    case 8:
        board[8] = 'O';
        break;
    case 9:
        board[9] = 'O';
        break;
    default:
            cout<<"Please make a valid selection.";
        }
        player2++;
        string clr(50, '\n');
        cout<<clr;
    }

else{
    cout<<"Woah something went terribly wrong! Program is closing!";
    return 0;
}
}
}

/*int selection(int selction){
    cout<<"Please make a selction: "
    cin>>selection;
    switch(selection)

*/


void board_draw(){
    char *b = board;

    cout<<"     |     |     "<<endl;
    cout<<"  "<<b[1]<<"  |"<<"  "<<b[2]<<"  |  "<<b[3]<<endl;
    cout<<"_____|_____|_____"<<endl;
    cout<<"     |     |     "<<endl;
    cout<<"  "<<b[4]<<"  |"<<"  "<<b[5]<<"  |  "<<b[6]<<endl;
    cout<<"_____|_____|_____"<<endl;
    cout<<"     |     |     "<<endl;
    cout<<"  "<<b[7]<<"  |"<<"  "<<b[8]<<"  |  "<<b[9]<<endl;
    cout<<"     |     |     "<<endl;


}


I also completely understand that most of this is noobish and in sure it's complete poop.
Dec 30, 2012 at 8:26am
Instead of using ints to check between player1 and player2 I would just use a single bool to check which players turn it currently is. You also don't need to check every number a user can enter with a case statement you could make it simpler with something like this.

1
2
3
4
5
6
7
8
9
cin >> selection;
if(selection >= 0 && selection <= 9) // Makes sure it is a valid number
{
	if(Player1)
		board[selection] = 'O';  // Inserts character into that space

	else
		board[selection] = 'X';
}


Then at the end of every turn you could use something this to change players.
1
2
3
4
5
if(Player1)  
	Player1 = false;

else
	Player1 = true;


There are several other ways to do it, but hopefully that gives you some ideas. (On a side note you are missing 0 on your case statements as arrays start at 0.)
Dec 30, 2012 at 10:03am
Does this look any better?

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
#include <iostream>


using namespace std;

void board_draw();//board drawing function

int game_check();//game check prototype

char board[10] = {'0','1','2','3','4','5','6','7','8','9'};//global array
int player1 = 1;//global bool for player


int main()
{
    int turns = 0;
        while(turns < 9)/*cheecks that turn does not exceed 9*/
        {
            game_check();
            board_draw();//Draw board by initilizing function
            int selection;

    if(player1 == 1) {//if player one is true then do this ie. is player 1
            cout<<"\nPlayer 1, Please make a move by selcting your desired space: ";
                cin>>selection;
                cout<<"\n\n\n";
            board[selection] = 'X';
            player1 = 2;
                        }
    else if(player1 == 2){//opposite of above
        cout<<"\nPlayer 2, Please make a move by selcting your desired space: ";
            cin>>selection;
            cout<<"\n\n\n";
        board[selection] = 'O';
        player1 = 1;
        }
        else{//if playe1 is set to 3 by function game_check, the program ends
            break;
        }
        turns++;
}
}


void board_draw(){
    char *b = board;

    cout<<"\n     |     |     "<<endl;
    cout<<"  "<<b[1]<<"  |"<<"  "<<b[2]<<"  |  "<<b[3]<<endl;
    cout<<"_____|_____|_____"<<endl;
    cout<<"     |     |     "<<endl;
    cout<<"  "<<b[4]<<"  |"<<"  "<<b[5]<<"  |  "<<b[6]<<endl;
    cout<<"_____|_____|_____"<<endl;
    cout<<"     |     |     "<<endl;
    cout<<"  "<<b[7]<<"  |"<<"  "<<b[8]<<"  |  "<<b[9]<<endl;
    cout<<"     |     |     "<<endl;


}

int game_check(){
 int game_check = -1;
if(board[1] == board[2] && board[2] == board[3]){
    game_check = 1;
}
else if(board[4] == board[5] && board[5] == board[6]){
    game_check = 1;
}
else if(board[7] == board[8] && board[8] == board[9]){
    game_check = 1;
}
else if(board[1] == board[4] && board[4] == board[7]){
    game_check = 1;
}
else if(board[2] == board[5] && board[5] == board[8]){
    game_check = 1;
}
else if(board[3] == board[6] && board[6] == board[9]){
    game_check = 1;
}
else if(board[1] == board[5] && board[5] == board[9]){
    game_check = 1;
}
else if(board[3] == board[5] && board[5] == board[7]){
    game_check = 1;
}
else{
    game_check = 0;
}

//if(game_check == 1){
 //   cout<<"AAAAAAAAAA";
//}
if(game_check == 1 && player1 == 2){
    cout<<"Player 1 Wins!!!";
    player1 = 3;//sets player1 to 3 to terminate program
}
else if(game_check == 1 && player1 == 1){
    cout<<"Player 2 Wins!!!";
    player1 = 3;//sets player1 to 3 to terminate program
}
}


I also inserted a game checking function which turned out ok. But it was interesting to find a way to close the program when when the one of the two different conditions i wanted to check was inside of a function.
Any more input would be greatly appreciated. Thank-You in advanced.
Dec 30, 2012 at 10:24am
Sorry, but here's my board (old) :
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
void board_draw()
{
system("cls");
char *b = board;
cout << "\n\n\tTic Tac Toe\n\n";
cout << "Player 1 (X) - Computer (O)" << endl << endl << endl;
cout << "____________" << endl;
cout << "|  |   |   |  " << endl;
cout << "|" << b[1] << " | " << b[2] << " | " << b[3] << " | " << endl;
cout << "____________" << endl;
cout << "|  |   |   |  " << endl;
cout << "|" << b[4] << " | " << b[5] << " | " << b[6] <<  " | " << endl;
cout << "____________" << endl;
cout << "|  |   |   |  " << endl;
cout << "|" << b[7] << " | " << b[8] << " | " << b[9] <<  " | " << endl;
cout << "____________" << endl;
}


when the one of the two different conditions i wanted to check was inside of a function.

Be more detailed? (You may explain or post an example here...)
Last edited on Dec 30, 2012 at 10:25am
Dec 30, 2012 at 9:07pm
I wanted to take the variable from my function
1
2
game_check();
int game_check;

and use it as a variable in my while loop. Like so.
1
2
3
while((game_check != 1 ) && (turns < 9)){

}

So that whenever int game_check(); did check that one of the players had one (hehe it's kind of punny) the program would end.
Last edited on Dec 30, 2012 at 9:08pm
Dec 30, 2012 at 11:57pm
Firstly, remove int game_check;. It's duplicate (name).

You should return value - because assigning a function with a value is totally wrong (Did you look at the compling error log?)

return 1;

Rather than :
game_check = 1;//error

And how to catch the result from the function game_check() :

1
2
3
int result = game_check();
if(result == 1){[...]} // win
else if (result == 0){[...]} // lose 


Listen to the compiler. When you had any trouble just ask.
Last edited on Dec 31, 2012 at 12:01am
Dec 31, 2012 at 3:43am
> Firstly, remove int game_check;. It's duplicate (name).

It is bad style; but it is not a duplicate name - the scopes are different.


> I wanted to take the variable from my function
> and use it as a variable in my while loop

Have the function return the value.

Also, prefer bool over int for predicates (either true or false).

The program logic is the same as before, but with some re-factoring:

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
#include <iostream>
using namespace std ;

void draw_board() ;
bool game_over() ; // returns true or false

char board[10] = {'0','1','2','3','4','5','6','7','8','9'} ;
bool player1_next = true ; // true if player1 is next to play

int main()
{
    int turns = 0 ;
    draw_board() ;

    while( turns < 9 && !game_over() )
    {
        int selection ;
        cout << "\nPlayer " << ( player1_next ? 1 : 2 ) << " please make a move\n" ;

        if( cin >> selection && selection > 0 && selection < 10 )
        {
            board[selection] = player1_next ? 'X' : '0' ;
            ++turns ;
            player1_next = !player1_next ; // switch next to play
            draw_board() ;
        }
        else // handle incorrect input
        {
            cin.clear() ; // clear a possible error state
            cin.ignore( 1000, '\n' ) ; // and empty the input buffer
            cout << "*** invalid move: " ;
        }
    }

     if( turns < 9 ) std::cout << "\nPlayer " << ( player1_next ? 2 : 1 ) << " wins.\n" ;
     else std::cout << "\nIt's a tie.\n" ;
}

void draw_row( int row )
{
    if( row == 0 || row == 8 ) cout << "\n     |     |     \n" ;
    else if( row == 2 || row == 5 ) cout << "_____|_____|_____\n" ;
    else if( row == 3 || row == 6 ) cout << "     |     |     \n" ;
    else cout<< "  " << board[row] << "  |  " << board[row+1] << "  |  " << board[row+2] << '\n' ;
}

void draw_board()
{
    cout << "\n\n\n" ;
    for( int i = 0 ; i < 9 ; ++i ) draw_row(i) ;
}

bool game_over()
{
    for( int i = 1 ; i < 10 ; i += 3 ) // check rows
        if( board[i] == board[i+1] && board[i] == board[i+2] ) return true ;

    for( int i = 1 ; i < 4 ; ++i ) // check cols
        if( board[i] == board[i+3] && board[i] == board[i+6] ) return true ;

    // check diagonals
    if( board[1] == board[5] && board[1] == board[9] ) return true ;
    if( board[3] == board[5] && board[3] == board[7] ) return true ;

    return false ;
}


This won't prevent a player from cheating; for instance right at the first turn, player one can enter
Player 1 please make a move
1 4 2 5 3

and win before player two has had a chance.

(To take care of that, we need to empty the input buffer after reading just one integer).
Jan 2, 2013 at 12:50pm
I really appreciate all of the input! Some of if I don't necessarily understand completely but im trying. Here is my latest addition, adding a computer option and adding a (very) simple AI.

Well here is half of it. Im sure it's way longer than it needs to be. What do you guys think?

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
#include <iostream>
#include <ctime>
#include <cstdlib>

using namespace std;

void board_draw();//board drawing function

int game_check();//game check prototype

int pvp();//Player vs player function prototype

int pvc();//player vs computer prototype

int computer_ai(int s);//computer artificial inteligence function prototype

char board[10] = {'0','1','2','3','4','5','6','7','8','9'};//global array
int player1 = 1;//global int for player

int main()
{
    char computer_or_player;
    cout<<"Would you like to play against the computer? [y]es or [n]o : ";
    cin>>computer_or_player;
    switch(computer_or_player){
        case 'n':
             pvp();
            break;

        case 'y':
             pvc();

    }

}

void board_draw(){//we are just designing the board here
    char *b = board;
    cout<<"Welcome to Derrik's Tic Tac Toe Experience!";
    cout<<"\n     |     |     "<<endl;
    cout<<"  "<<b[1]<<"  |"<<"  "<<b[2]<<"  |  "<<b[3]<<endl;
    cout<<"_____|_____|_____"<<endl;
    cout<<"     |     |     "<<endl;
    cout<<"  "<<b[4]<<"  |"<<"  "<<b[5]<<"  |  "<<b[6]<<endl;
    cout<<"_____|_____|_____"<<endl;
    cout<<"     |     |     "<<endl;
    cout<<"  "<<b[7]<<"  |"<<"  "<<b[8]<<"  |  "<<b[9]<<endl;
    cout<<"     |     |     "<<endl;


}

int game_check(){//our game checing function
 int game_check = -1;//sets game_check to -1 which means it won't effect anything
if(board[1] == board[2] && board[2] == board[3]){
    game_check = 1;
}
else if(board[4] == board[5] && board[5] == board[6]){
    game_check = 1;
}
else if(board[7] == board[8] && board[8] == board[9]){
    game_check = 1;
}
else if(board[1] == board[4] && board[4] == board[7]){//Checking all possible winning outcomes of this 3 x 3 tic tac toe game.
    game_check = 1;
}
else if(board[2] == board[5] && board[5] == board[8]){
    game_check = 1;
}
else if(board[3] == board[6] && board[6] == board[9]){
    game_check = 1;
}
else if(board[1] == board[5] && board[5] == board[9]){
    game_check = 1;
}
else if(board[3] == board[5] && board[5] == board[7]){
    game_check = 1;
}
else{
    game_check = 0;
}

if(game_check == 1 && player1 == 2){/*If the game has been won then this decides who won. while player1 = 2 Player 1 has won the game.
        because game check is at the begining of the loop. so the value for player 2 to equal "true" is set before it checks the game
        and i was to lazy to move it 8)*/
    cout<<"Player 1 Wins!!!\n\n";
    player1 = 3;//sets player1 to 3 to terminate program
}
else if(game_check == 1 && player1 == 1){
    cout<<"Player 2 Wins!!!\n\n";
    player1 = 3;//sets player1 to 3 to terminate program
}
}

int pvp()
{
    cout<<"hello";
    int turns = 0;
while(turns < 9)/*cheecks that turn does not exceed 9*/
        {
            game_check();
            board_draw();//Draw board by initilizing function
            int selection;

    if(player1 == 1) {//if player one is true then do this ie. is player 1
            cout<<"\nPlayer 1, Please make a move by selcting your desired space: ";
                 if( cin >> selection && selection > 0 && selection < 10 )
        {
            cin.ignore(1000, '\n');
             if((board[selection] == 'X') || (board[selection] == 'O'))//checks to ensure that the space has not already been taken
                {
                cout<<"Please pick a spot that has not been taken";
             }
             else{//If space had not been taken already then manipulate it
            board[selection] = 'X';
            player1 = 2;
            turns++;
             }
        }
        else{
                 cin.clear() ; // clear a possible error state
            cin.ignore( 1000, '\n' ) ; // and empty the input buffer
            cout << "*** invalid move \n\n " ;

        }
                        }


    else if(player1 == 2){//opposite of above
        cout<<"\nPlayer 2, Please make a move by selcting your desired space: ";
              if( cin >> selection && selection > 0 && selection < 10 )
        {
            cin.ignore(1000, '\n');
            if((board[selection] == 'X') || (board[selection] == 'O'))//checks to ensure that the space has not already been taken
                {
                cout<<"Please pick a spot that has not been taken";
             }
             else{
        board[selection] = 'O';
        player1 = 1;
        turns++;
             }
        }
        else{
                 cin.clear() ; // clear a possible error state
            cin.ignore( 1000, '\n' ) ; // and empty the input buffer
            cout << "*** invalid move\n\n " ;

        }
Jan 2, 2013 at 12:51pm
And the other half....

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
        }
        else{//if player1 is set to three by game_break function the grogram breaks out of loop
            break;
        }
}
}

int pvc()
{
    int turns = 0;
    int s;

               while(turns < 9)/*cheecks that turn does not exceed 9*/
        {
            game_check();
            board_draw();//Draw board by initilizing function
            int selection;

    if(player1 == 1) {//if player one is true then do this ie. is player 1
            cout<<"\nPlayer 1, Please make a move by selcting your desired space: ";
                if(cin>> selection && selection > 0 && selection < 10){
                        cin.ignore(1000, '\n');
             if((board[selection] == 'X') || (board[selection] == 'O'))//checks to ensure that the space has not already been taken
                {
                cout<<"Please pick a spot that has not been taken";
             }
             else{//If space had not been taken already then manipulate it
            board[selection] = 'X';
            player1 = 2;
            turns++;
             }
                        }

                else{
                     cin.clear() ; // clear a possible error state
            cin.ignore( 1000, '\n' ) ; // and empty the input buffer
            cout << "*** invalid move\n\n " ;
                }
    }

    else if(player1 == 2){//opposite of above
        cout<<"\n  ";
          s = computer_ai(s);
                selection = s;
            cout<<"\n\n\n";
            if((board[selection] == 'X') || (board[selection] == 'O'))//checks to ensure that the space has not already been taken
                {
                cout<<"Please pick a spot that has not been taken";
             }
             else{
        board[selection] = 'O';
        player1 = 1;
        turns++;
             }
        }
        else{//if player1 is set to three by game_break function the grogram breaks out of loop
            break;
        }
    }

}

int computer_ai(int s)
{
    if(board[1] == 'X' && board[2] == 'X' && board[3] == '3'){
        s = 3;
    }

    else if(board[1] == 'X' && board[3] == 'X' && board[2] == '2'){
        s = 2;
    }

     else if(board[1] == 'X' && board[4] == 'X' && board[7] == '7'){
        s = 7;
    }

     else if(board[1] == 'X' && board[7] == 'X' && board[4] == '4'){
        s = 4;
    }

     else if(board[1] == 'X' && board[5] == 'X' && board[9] == '9'){
        s = 9;
    }

     else if(board[1] == 'X' && board[9] == 'X' && board[5] == '5'){
        s = 5;
    }

         else if(board[2] == 'X' && board[3] == 'X' && board[1] == '1'){
        s = 1;
    }

         else if(board[2] == 'X' && board[5] == 'X' && board[8] == '8'){
        s = 8;
    }

         else if(board[2] == 'X' && board[8] == 'X' && board[5] == '5'){
        s = 5;
    }

         else if(board[3] == 'X' && board[6] == 'X' && board[9] == '9'){
        s = 9;
    }

         else if(board[3] == 'X' && board[9] == 'X' && board[6] == '6'){
        s = 6;
    }

         else if(board[3] == 'X' && board[5] == 'X' && board[7] == '7'){
        s = 7;
    }

         else if(board[3] == 'X' && board[7] == 'X' && board[5] == '5'){
        s = 5;
    }

         else if(board[4] == 'X' && board[7] == 'X' && board[1] == '1'){
        s = 1;
    }

     else if(board[4] == 'X' && board[5] == 'X' && board[6] == '6'){
        s = 6;
    }

     else if(board[4] == 'X' && board[6] == 'X' && board[5] == '5'){
        s = 1;
    }

     else if(board[5] == 'X' && board[8] == 'X' && board[2] == '2'){
        s = 2;
    }

     else if(board[5] == 'X' && board[9] == 'X' && board[1] == '1'){
        s = 1;
    }

     else if(board[5] == 'X' && board[7] == 'X' && board[3] == '3'){
        s = 3;
    }

     else if(board[5] == 'X' && board[6] == 'X' && board[4] == '4'){
        s = 4;
     }

     else if(board[6] == 'X' && board[9] == 'X' && board[3] == '3'){
        s = 3;
    }

     else if(board[7] == 'X' && board[8] == 'X' && board[9] == '9'){
        s = 9;
    }

     else if(board[7] == 'X' && board[9] == 'X' && board[8] == '8'){
        s = 8;
    }

     else if(board[8] == 'X' && board[9] == 'X' && board[7] == '7'){
        s = 7;
    }

    else{
            srand(time(NULL));
        s = (rand() % 8 + 1);
    }

return s;


}
Topic archived. No new replies allowed.