if statement does not make correct decision

Pages: 12
Hi i recently wrote a "Tic Tac Toe" console game, and i seem to have a problem in my winning conditions, as when it checks if the player won (should have won) it doesn't take the appropriate action.

The winning conditions are in CoreLogic.cpp

here's the code, might not be the most pretty and clean code ever but it works for me.

there are more files i just didn't feel the need to include them all since the problem is only within CoreLogic.cpp and possibly in main.cpp

PS: pos1-pos9 have been declared in the UserInterface.cpp and are being used to determine the correct decision to do within CoreLogic.cpp

PPS: Nothing happens when you win at this point it just exits the program.
Last edited on
main.cpp
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
#include <iostream>
#include <string>
#include "UserInterface.h"
#include "CoreLogic.h"

using namespace std;

int main()
{
    //Class Objects
    UserInterface UI;
    CoreLogic LOGIC;

    //bool
    bool quit = false;

    //string
    string player1 = "";
    string player2 = "";

    //char
    char input;

    //int
    int player_1 = 1;
    int player_2 = 2;
    int position = 0;
    int win = LOGIC.logic();

    //Title
    cout << "\t\t\t        Tic Tac Toe V1.0" << endl;
    cout << endl << endl << endl << endl;

    //Greets and gets player1's name
    cout << "Welcome Player 1 please enter your first name: ";
    cin >> player1;
    cout << "Nice to meet you " << player1 << "." << endl << endl << endl;

    //Greets and gets player2's name
    cout << "Welcome Player 2 please enter your first name: ";
    cin >> player2;
    cout << "Nice to meet you " << player2 << "." << endl << endl << endl;

    Start:

    //gets user input on how to continue.
    cout << "Would you like to start a new game? Y/N >> ";
    cin >> input;
    cout << endl << endl << endl;
    input = toupper(input);

    if(input == 'Y')
    {
        cout << string(100, '\n');
    }
    else if(input == 'N')
    {
        return 0;
    }
    else
    {
        cout << "You have entered an unrecognized option please try again" << endl << endl;
        return 0;
    }


    Game_Start:

    cout << "X's start that's you " << player1 << endl << endl << endl;
    UI.update();
    do
    {
        //Gets player1's position
        cout << player1 << " Please enter your position 1-9 >> ";
        cin >> position;

        UI.playerPos(position, player_1);
        UI.update();
        LOGIC.logic();

        //Checks if the move performed is a winning move
        if(win == 0)
        {
            quit = false;
        }
        else if(win == 1)
        {
            quit = true;
        }
        else if(win == 2)
        {
            quit = true;
        }

        //Resets the pos variable to 0
        position = 0;


        //Gets player2's position
        cout << player2 << " Please enter your position 1-9 >> ";
        cin >> position;

        UI.playerPos(position, player_2);
        UI.update();
        win = LOGIC.logic();

        //Checks if the move performed is a winning move
        if(win == 0)
        {
            quit = false;
        }
        else if(win == 1)
        {
            quit = true;
        }
        else if(win == 2)
        {
            quit = true;
        }

        //Resets the pos variable to 0
        position = 0;


    }while(quit == false);


    return 0;
}

Last edited on
CoreLogic.cpp

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
#include "CoreLogic.h"
#include "UserInterface.h"
#include <iostream>
#include <string>

using namespace std;


CoreLogic::CoreLogic()
{
    //ctor
}

int CoreLogic::logic()
{
    //class object
    UserInterface POS;

    //int
    int winner = 0;

    //char
    char pos1 = POS.pos1;
    char pos2 = POS.pos2;
    char pos3 = POS.pos3;
    char pos4 = POS.pos4;
    char pos5 = POS.pos5;
    char pos6 = POS.pos6;
    char pos7 = POS.pos7;
    char pos8 = POS.pos8;
    char pos9 = POS.pos9;


    cout << pos1 << pos2 << pos3 << pos4 << pos5 << pos6 << pos7 << pos8 << pos9 << endl;



    //Win possibilities for player 1(X)

    //Bottom Horizontal
    if(pos1 == 'X' && pos2 == 'X' && pos3 == 'X')
    {
         winner = 1;
    }
    //Middle Horizontal
    else if(pos4 == 'X' && pos5 == 'X' && pos6 == 'X')
    {
        winner = 1;
    }
    //Top Horizontal
    else if(pos7 == 'X' && pos8 == 'X' && pos9 == 'X')
    {
        winner = 1;
    }
    //Left Vertical
    else if(pos1 == 'X' && pos4 == 'X' && pos7 == 'X')
    {
        winner = 1;
    }
    //Middle Vertical
    else if(pos2 == 'X' && pos5 == 'X' && pos8 == 'X')
    {
        winner = 1;
    }
    //Right Vertical
    else if(pos3 == 'X' && pos6 == 'X' && pos9 == 'X')
    {
        winner = 1;
    }
    //Bottom Left - Top Right Diagonal
    else if(pos1 == 'X' && pos5 == 'X' && pos9 == 'X')
    {
        winner = 1;
    }
    //Bottom Right - Top Left Diagonal
    else if(pos3 == 'X' && pos5 == 'X' && pos7 == 'X')
    {
        winner = 1;
    }
    //Player 1 has not won.

    //Win possibilities for player 2(O)

    //Bottom Horizontal
    if(pos1 == 'O' && pos2 == 'O' && pos3 == 'O')
    {
        winner = 2;
    }
    //Middle Horizontal
    else if(pos4 == 'O' && pos5 == 'O' && pos6 == 'O')
    {
        winner = 2;
    }
    //Top Horizontal
    else if(pos7 == 'O' && pos8 == 'O' && pos9 == 'O')
    {
        winner = 2;
    }
    //Left Vertical
    else if(pos1 == 'O' && pos4 == 'O' && pos7 == 'O')
    {
        winner = 2;
    }
    //Middle Vertical
    else if(pos2 == 'O' && pos5 == 'O' && pos8 == 'O')
    {
        winner = 2;
    }
    //Right Vertical
    else if(pos3 == 'O' && pos6 == 'O' && pos9 == 'O')
    {
        winner = 2;
    }
    //Bottom Left - Top Right Diagonal
    else if(pos1 == 'O' && pos5 == 'O' && pos9 == 'O')
    {
        winner = 2;
    }
    //Bottom Right - Top Left Diagonal
    else if(pos3 == 'O' && pos5 == 'O' && pos7 == 'O')
    {
        winner = 2;
    }
    //Player 2 has not won.

    //returns 0-2
    return (winner);
}

Last edited on
steleb wrote:
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
        //Checks if the move performed is a winning move
        if(win == 0)
        {
            quit = false;
        }
        else if(win == 1)
        {
            goto winner;
            quit = true;
        }
        else if(win == 2)
        {
            goto winner;
            quit = true;
        }
        winner:
goto is bad practice. It is especially bad here because it is the source of your problem. Even if it were not the source of your problem, its use here is pointless.
Last edited on
goto is bad practice. It is especially bad here because it is the source of your problem. Even if it were not the source of your problem, its use here is pointless.


i removed all the goto, and the problem still persists i also realised i had a extra #include removed it but still wasnt the problem
Last edited on
That's quite a significant change to make considering how much you used it in other places - can you post the current code you have?
That's quite a significant change to make considering how much you used it in other places - can you post the current code you have?


i updated the main.cpp above

without the goto's it will ask player 2 to input a new position even though player 1 may have already won...i can fix this at some point, but more worried about my bigger problem here.
Last edited on
I only intended for you to remove the gotos in the section of code I quoted - if you are on a time crunch you should leave the rest of your code alone.
i'm just making this program to learn more about c++ and programming overall... i'm going to post both my UserInterface.cpp and UserInterface.h because i now think those file might be my problem.


i alse added cout << pos1 << pos2 << pos3 << pos4 << pos5 << pos6 << pos7 << pos8 << pos9 << endl;

for debugging purposes


i also updated CoreLogic.cpp
Last edited on
UserInterface.h

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
#ifndef USERINTERFACE_H
#define USERINTERFACE_H

using namespace std;

class UserInterface
{
    public:
        UserInterface();

        //gets and sets the player posisiton
        char playerPos(int pos, int player);

        //updates the play area
        void update();

        char pos1 = '1';
        char pos2 = '2';
        char pos3 = '3';
        char pos4 = '4';
        char pos5 = '5';
        char pos6 = '6';
        char pos7 = '7';
        char pos8 = '8';
        char pos9 = '9';

};

#endif // USERINTERFACE_H 
UserInterface.cpp

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
#include "UserInterface.h"
#include <iostream>

using namespace std;






//Constructor
UserInterface::UserInterface()
{
}

//sets the player postition
//checks if position the player chose is not already filled
char UserInterface::playerPos(int pos, int player)  //player 1 = 'X'
{                                                   //player 2 = "0"

    //bool
    bool correctPos = true;



    //checks if the player entered a recognized position
    do
    {
        if(pos < 1 || pos > 9)
        {
            cout << "The position you have entered is not recognizable.\nPlease enter another position. >> ";
            cin >> pos;
            cout << endl;
        }
        else{correctPos = false;}


    }while(correctPos == true);

    //checks if the position the player chose is not already filled
    //if the position is free X/O will be set for that position
    do
    {
        correctPos = true;

         if(player == 1)
         {
             switch (pos)
             {
             case 1:

                if(pos1 == 'X' || pos1 == 'O')
                {
                    cout << "You have entered a position that has already been filled.\nPlease enter another position. >> ";
                    cin >> pos;
                    cout << endl;

                    break;
                }
                else
                {
                    pos1 = 'X';

                    correctPos = false;
                    return pos1, pos2, pos3, pos4, pos5, pos6, pos7, pos8, pos9;
                    break;
                }

             case 2:

                if(pos2 == 'X' || pos2 == 'O')
                {
                    cout << "You have entered a position that has already been filled.\nPlease enter another position. >> ";
                    cin >> pos;
                    cout << endl;

                    break;
                }
                else
                {
                    pos2 = 'X';
                    correctPos = false;
                    return pos1, pos2, pos3, pos4, pos5, pos6, pos7, pos8, pos9;
                    break;
                }

             case 3:

                if(pos3 == 'X' || pos3 == 'O')
                {
                    cout << "You have entered a position that has already been filled.\nPlease enter another position. >> ";
                    cin >> pos;
                    cout << endl;

                    break;
                }
                else
                {
                    pos3 = 'X';
                    correctPos = false;
                    return pos1, pos2, pos3, pos4, pos5, pos6, pos7, pos8, pos9;
                    break;
                }

             case 4:

                if(pos4 == 'X' || pos4 == 'O')
                {
                    cout << "You have entered a position that has already been filled.\nPlease enter another position. >> ";
                    cin >> pos;
                    cout << endl;

                    break;
                }
                else
                {
                    pos4 = 'X';
                    correctPos = false;
                    return pos1, pos2, pos3, pos4, pos5, pos6, pos7, pos8, pos9;
                    break;
                }

             case 5:

                if(pos5 == 'X' || pos5 == 'O')
                {
                    cout << "You have entered a position that has already been filled.\nPlease enter another position. >> ";
                    cin >> pos;
                    cout << endl;

                    break;
                }
                else
                {
                    pos5 = 'X';
                    correctPos = false;
                    return pos1, pos2, pos3, pos4, pos5, pos6, pos7, pos8, pos9;
                    break;
                }

             case 6:

                if(pos6 == 'X' || pos6 == 'O')
                {
                    cout << "You have entered a position that has already been filled.\nPlease enter another position. >> ";
                    cin >> pos;
                    cout << endl;

                    break;
                }
                else
                {
                    pos6 = 'X';
                    correctPos = false;
                    return pos1, pos2, pos3, pos4, pos5, pos6, pos7, pos8, pos9;
                    break;
                }

             case 7:

                if(pos7 == 'X' || pos7 == 'O')
                {
                    cout << "You have entered a position that has already been filled.\nPlease enter another position. >> ";
                    cin >> pos;
                    cout << endl;

                    break;
                }
                else
                {
                    pos7 = 'X';
                    correctPos = false;
                    return pos1, pos2, pos3, pos4, pos5, pos6, pos7, pos8, pos9;
                    break;
                }

             case 8:

                if(pos8 == 'X' || pos8 == 'O')
                {
                    cout << "You have entered a position that has already been filled.\nPlease enter another position. >> ";
                    cin >> pos;
                    cout << endl;

                    break;
                }
                else
                {
                    pos8 = 'X';
                    correctPos = false;
                    return pos1, pos2, pos3, pos4, pos5, pos6, pos7, pos8, pos9;
                    break;
                }

             case 9:

                if(pos9 == 'X' || pos9 == 'O')
                {
                    cout << "You have entered a position that has already been filled.\nPlease enter another position. >> ";
                    cin >> pos;
                    cout << endl;

                    break;
                }
                else
                {
                    pos9 = 'X';
                    correctPos = false;
                    return pos1, pos2, pos3, pos4, pos5, pos6, pos7, pos8, pos9;
                    break;
                }

             }
         }
UserInterface.cpp ...Continued

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
 else if(player == 2)
        {
            switch (pos)
             {
             case 1:

                if(pos1 == 'X' || pos1 == 'O')
                {
                    cout << "You have entered a position that has already been filled.\nPlease enter another position. >> ";
                    cin >> pos;
                    cout << endl;

                    break;
                }
                else
                {
                    pos1 = 'O';
                    correctPos = false;
                    return pos1, pos2, pos3, pos4, pos5, pos6, pos7, pos8, pos9;
                    break;
                }

             case 2:

                if(pos2 == 'X' || pos2 == 'O')
                {
                    cout << "You have entered a position that has already been filled.\nPlease enter another position. >> ";
                    cin >> pos;
                    cout << endl;

                    break;
                }
                else
                {
                    pos2 = 'O';
                    correctPos = false;
                    return pos1, pos2, pos3, pos4, pos5, pos6, pos7, pos8, pos9;
                    break;
                }

             case 3:

                if(pos3 == 'X' || pos3 == 'O')
                {
                    cout << "You have entered a position that has already been filled.\nPlease enter another position. >> ";
                    cin >> pos;
                    cout << endl;

                    break;
                }
                else
                {
                    pos3 = 'O';
                    correctPos = false;
                    return pos1, pos2, pos3, pos4, pos5, pos6, pos7, pos8, pos9;
                    break;
                }

             case 4:

                if(pos4 == 'X' || pos4 == 'O')
                {
                    cout << "You have entered a position that has already been filled.\nPlease enter another position. >> ";
                    cin >> pos;
                    cout << endl;

                    break;
                }
                else
                {
                    pos4 = 'O';
                    correctPos = false;
                    return pos1, pos2, pos3, pos4, pos5, pos6, pos7, pos8, pos9;
                    break;
                }

             case 5:

                if(pos5 == 'X' || pos5 == 'O')
                {
                    cout << "You have entered a position that has already been filled.\nPlease enter another position. >> ";
                    cin >> pos;
                    cout << endl;

                    break;
                }
                else
                {
                    pos5 = 'O';
                    correctPos = false;
                    return pos1, pos2, pos3, pos4, pos5, pos6, pos7, pos8, pos9;
                    break;
                }

             case 6:

                if(pos6 == 'X' || pos6 == 'O')
                {
                    cout << "You have entered a position that has already been filled.\nPlease enter another position. >> ";
                    cin >> pos;
                    cout << endl;

                    break;
                }
                else
                {
                    pos6 = 'O';
                    correctPos = false;
                    return pos1, pos2, pos3, pos4, pos5, pos6, pos7, pos8, pos9;
                    break;
                }

             case 7:

                if(pos7 == 'X' || pos7 == 'O')
                {
                    cout << "You have entered a position that has already been filled.\nPlease enter another position. >> ";
                    cin >> pos;
                    cout << endl;

                    break;
                }
                else
                {
                    pos7 = 'O';
                    correctPos = false;
                    return pos1, pos2, pos3, pos4, pos5, pos6, pos7, pos8, pos9;
                    break;
                }

             case 8:

                if(pos8 == 'X' || pos8 == 'O')
                {
                    cout << "You have entered a position that has already been filled.\nPlease enter another position. >> ";
                    cin >> pos;
                    cout << endl;

                    break;
                }
                else
                {
                    pos8 = 'O';
                    correctPos = false;
                    return pos1, pos2, pos3, pos4, pos5, pos6, pos7, pos8, pos9;
                    break;
                }

             case 9:

                if(pos9 == 'X' || pos9 == 'O')
                {
                    cout << "You have entered a position that has already been filled.\nPlease enter another position. >> ";
                    cin >> pos;
                    cout << endl;

                    break;
                }
                else
                {
                    pos9 = 'O';
                    correctPos = false;
                    return pos1, pos2, pos3, pos4, pos5, pos6, pos7, pos8, pos9;
                    break;
                }

             }
        }

    }while(correctPos == true);
}

//updates the play area
void UserInterface::update()
{
    cout << endl;
    cout << "  " << pos7 << " | " << pos8 << " | " << pos9 << " " << endl;
    cout << " ----------" << endl;
    cout << "  " << pos4 << " | " << pos5 << " | " << pos6 << " " <<endl;
    cout << " ----------" << endl;
    cout << "  " << pos1 << " | " << pos2 << " | " << pos3 << " " << endl;
    cout << endl;
}
Last edited on
return pos1, pos2, pos3, pos4, pos5, pos6, pos7, pos8, pos9;

could be rewritten as:

return pos9;

could be rewritten as:

return pos9;


well not really since each of those variables have different values. pos1 is not the same as pos2, pos3 ...
The comma operator in C++ is a special operator that evaluates all the expressions and returns only the result of the last expression. The results of all other expressions are discarded.

int x = (1, 2, 3);

x has value 3.
The comma operator in C++ is a special operator that evaluates all the expressions and returns only the result of the last expression. The results of all other expressions are discarded.

int x = (1, 2, 3);

x has value 3.


I don't see how this affects me, could you explain how it does?
It is the same thing that cire pointed out:
http://www.cplusplus.com/forum/general/157167/#msg806292
It is the same thing that cire pointed out:
http://www.cplusplus.com/forum/general/157167/#msg806292


I understand that's what you guys are talking about, what i made an array and returned that array would that work? because i need all those 9 values, or is there a better way of doing this?

I've come to the conclusion that my problem is how i return these 9 values, but now my question is what can i do to solve my problem?
understand that's what you guys are talking about, what i made an array and returned that array would that work? because i need all those 9 values, or is there a better way of doing this?


You have 9 variables which are a part of (the public interface of) your class object. Their value persists across calls to the function. There is no reason to return them.

What are you using the return value from the function for?
i was only using the return function because the variables wouldn't get updated when i called them in CoreLogic.cpp for example pos1 is equal to '1' when it really should be 'X' or 'O' this is the same for all 9 variables.

the return function is only there to try to fix the problem which it didnt exactly fix.
Pages: 12