C++

Forum

 
If Statement Not Working

Pages: 12
Devynao (30)
Every time I enter 1,2,3 it still gives me the cout for the first one.

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

void MainIntro() {
	cout << "Welcome To Mist Light" << endl << endl << endl;

	system("PAUSE");
	system("CLS");
}

void MainMenu() {
	int MainMenuChoice;

	cout << "[1] Play     [2] Credits     [3] Exit" << endl;
	cout << "    ^              ^              ^" << endl;
	cout << "     ^             ^             ^" << endl;
	cout << "      ^            ^            ^" << endl;
	cout << "       ^           ^           ^" << endl;
	cout << "        ^__________^__________^" << endl;
	cout << "                   ^" << endl;
	cout << "                   ^" << endl;
	cout << "                  ";
	cin >> MainMenuChoice;
}

void CharacterCreation() {

}

int main() {
	MainIntro();
	
	int MainMenuChoice = 4;
	MainMenu();
		if (MainMenuChoice == 1) {
			cout << "Your Chose to Play";
			system("PAUSE");
			system("CLS");
		}
		else if (MainMenuChoice == 2) {
			cout << "You chose credits.";
			system("PAUSE");
			system("CLS");
		}
		else if (MainMenuChoice == 3) {
			cout << "You chose to exit.";
			system("PAUSE");
			system("CLS");
		}

	

	return 0;
}
joe864864 (249)
You have MainMenuChoice initialized to 4. Then you call MainMenu which gives the user the opportunity to input their choice, but you do not pass their choice back to main. 2 options to fix it. One you can change the function from a void to an int and return MainMenuChoice. Two you can pass MainMenuChoice by reference.
Last edited on
Devynao (30)
How exactly do I return MainMenuChoice, or pass it by reference?
joe864864 (249)
Changes are commented. This is just using return.

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

void MainIntro() {
	cout << "Welcome To Mist Light" << endl << endl << endl;

	system("PAUSE");
	system("CLS");
}

int MainMenu() {//changed from a void function to an int
	int MainMenuChoice;

	cout << "[1] Play     [2] Credits     [3] Exit" << endl;
	cout << "    ^              ^              ^" << endl;
	cout << "     ^             ^             ^" << endl;
	cout << "      ^            ^            ^" << endl;
	cout << "       ^           ^           ^" << endl;
	cout << "        ^__________^__________^" << endl;
	cout << "                   ^" << endl;
	cout << "                   ^" << endl;
	cout << "                  ";
	cin >> MainMenuChoice;

	return MainMenuChoice;//added a return
}

void CharacterCreation() {

}

int main() {
	MainIntro();

	int MainMenuChoice = 4;
	MainMenuChoice = MainMenu(); //set MainMenuChoice to equal return of MainMenu function
	if (MainMenuChoice == 1) {
		cout << "Your Chose to Play";
		system("PAUSE");
		system("CLS");
	}
	else if (MainMenuChoice == 2) {
		cout << "You chose credits.";
		system("PAUSE");
		system("CLS");
	}
	else if (MainMenuChoice == 3) {
		cout << "You chose to exit.";
		system("PAUSE");
		system("CLS");
	}



	return 0;
}


Pass by reference:

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

void MainIntro() {
	cout << "Welcome To Mist Light" << endl << endl << endl;

	system("PAUSE");
	system("CLS");
}

void MainMenu(int &MainMenuChoice) {//change MainMenuChoice to be passed by reference

	cout << "[1] Play     [2] Credits     [3] Exit" << endl;
	cout << "    ^              ^              ^" << endl;
	cout << "     ^             ^             ^" << endl;
	cout << "      ^            ^            ^" << endl;
	cout << "       ^           ^           ^" << endl;
	cout << "        ^__________^__________^" << endl;
	cout << "                   ^" << endl;
	cout << "                   ^" << endl;
	cout << "                  ";
	cin >> MainMenuChoice;
}

void CharacterCreation() {

}

int main() {
	MainIntro();

	int MainMenuChoice = 4;
	MainMenu(MainMenuChoice);//added MainMenuChoice
	if (MainMenuChoice == 1) {
		cout << "Your Chose to Play";
		system("PAUSE");
		system("CLS");
	}
	else if (MainMenuChoice == 2) {
		cout << "You chose credits.";
		system("PAUSE");
		system("CLS");
	}
	else if (MainMenuChoice == 3) {
		cout << "You chose to exit.";
		system("PAUSE");
		system("CLS");
	}



	return 0;
}
closed account (48T7M4Gy)
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
 
#include <iostream>
#include <string>

using namespace std;

void display_Intro() {
    cout << "Welcome To Mist Light" << endl << endl << endl;
}

void display_Menu() {
    cout << "[1] Play     [2] Credits     [3] Exit" << endl;
    cout << "    ^              ^              ^" << endl;
    cout << "     ^             ^             ^" << endl;
    cout << "      ^            ^            ^" << endl;
    cout << "       ^           ^           ^" << endl;
    cout << "        ^__________^__________^" << endl;
    cout << "                   ^" << endl;
    cout << "                   ^" << endl;
    cout << "                  ";
}

int get_menu_selection()
{
    int selection = 0;
    
    while(cout << "Pls enter a selection: " and cin >> selection and (selection < 1 or selection > 3) )
    {
       cout << "Error\n";
    }
    return selection;
}

void CharacterCreation() {
    
}

int main() {
    
    int menu_selection = 0;
    
    display_Intro();
    display_Menu();
    menu_selection = get_menu_selection();
    
    if (menu_selection == 1) {
        cout << "Your Chose to Play";
    }
    else if (menu_selection == 2) {
        cout << "You chose credits.";
    }
    else
        cout << "You chose to exit.";
    
    return 0;
}
Devynao (30)
Thank you so much! I find it hard to learn from other peoples code, so you showing me with my code help a lot. I appreciate it. Are there any particular advantages doing it either way?
closed account (48T7M4Gy)
Are there any particular advantages doing it either way?


There's nothing greatly wrong with the way you were trying but I think it's simpler to break the functions down to simple (but still useful) blocks of code. I changed the names of functions and variables to ( I think better) reflect what's going on so you end up with almost self-documenting code.

The while loop for the selection is reasonably complex so it's a good idea to separate that from the menu display which is very simple.

BTW: By breaking it down a bit you are quite often able to re-use the functions elsewhere in the program.
Last edited on
Devynao (30)
@kemort I fixed my code and I also added a goto function instead of doing the while statement you suggested. Does my code still come to the same result as yours. Is is okay to use goto in this situation?


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
 

#include <iostream>
#include <string>
using namespace std;

void MainIntro() {
	cout << "Welcome To Mist Light" << endl << endl << endl;

	system("PAUSE");
	system("CLS");
}

int MainMenu() {
	int MainMenuChoice;

	cout << "[1] Play     [2] Credits     [3] Exit" << endl;
	cout << "    ^              ^              ^" << endl;
	cout << "     ^             ^             ^" << endl;
	cout << "      ^            ^            ^" << endl;
	cout << "       ^           ^           ^" << endl;
	cout << "        ^__________^__________^" << endl;
	cout << "                   ^" << endl;
	cout << "                   ^" << endl;
	cout << "                   ";
	cin >> MainMenuChoice;
	return MainMenuChoice;
}

void CharacterCreation() {

}

int main() {
	MainIntro();
	
//Main Menu
mainmenu:
	int MainMenuChoice;
	MainMenuChoice = MainMenu();      
		if (MainMenuChoice == 1) {
			cout << "Your Chose to Play";
			system("PAUSE");
			system("CLS");
		}
		else if (MainMenuChoice == 2) {
			cout << "You chose credits.";
			system("PAUSE");
			system("CLS");
		}
		else if (MainMenuChoice == 3) {
			cout << "You chose to exit.";
			system("PAUSE");
			system("CLS");
		}
		while (MainMenuChoice < 1, MainMenuChoice >3) {
			cout << "Please enter a valid option." << endl;
			system("PAUSE");
			system("CLS");
				goto mainmenu;
		}
	

	return 0;
}
joe864864 (249)
The better way to do it is to pass by reference, 2 reasons. First, if you are using return you can only pass one variable back. In your case, you only have one so it works fine but if you have more than the one you have to pass by reference. Second, when you get larger programs it is better to pass by reference to save memory space. When passing a variable by reference you are actually only passing the memory address of the variable and not the variable itself which saves space in memory.
cire (8284)
First, if you are using return you can only pass one variable back.

Or a container, or a compound object... not a good reason. Pass a reference if it is required that you have a variable to pass in and it should be modified. If that isn't required, returning a value is probably a better option.


When passing a variable by reference you are actually only passing the memory address of the variable and not the variable itself which saves space in memory.

This may be a good reason or bad reason for specific situations (although that'll tend mostly towards the bad end of the spectrum.) As far as the general case goes, it's useless.
Last edited on
closed account (48T7M4Gy)
goto function instead of doing the while statement you suggested.

goto's are there to be used I suppose but it is not good programming practice, unless you are a 'victim' of old legacy code.

The primary reason for me using the while loop is there is a bit of user input error checking going on. It's not a complete error check but does a great deal of the checking. The user gets taken around until the selection fits the menu then returns 'from whence it came' with a feasible selection.

goto's can be made to do the same but it's better to take advantage of the higher level instructions available.


Does my code still come to the same result as yours.
Testing with lots of test cases is the way to go.

Is is okay to use goto in this situation?
Bottom line - not really :(
In short, instead of 'goto', think 'return' Like the comment in the following article, I've never used it and that would be in line with the vast amount of code posted on this site.

http://stackoverflow.com/questions/379172/use-goto-or-not
Last edited on
bird1234 (126)
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
 
#include <iostream>
#include <string>
using namespace std;

void MainIntro() {
	cout << "Welcome To Mist Light" << endl << endl << endl;

	system("PAUSE");
	system("CLS");
}

int MainMenu() // use int instead of void {
	int MainMenuChoice;

	cout << "[1] Play     [2] Credits     [3] Exit" << endl;
	cout << "    ^              ^              ^" << endl;
	cout << "     ^             ^             ^" << endl;
	cout << "      ^            ^            ^" << endl;
	cout << "       ^           ^           ^" << endl;
	cout << "        ^__________^__________^" << endl;
	cout << "                   ^" << endl;
	cout << "                   ^" << endl;
	cout << "                  ";
	cin >> MainMenuChoice;
	return MainMenuChoice;
}

void CharacterCreation() {

}

int main() {
	MainIntro();

	int MainMenuChoice = 4;
	MainMenuChoice = MainMenu(); // return value will be store and no problem in if          //statement
	if (MainMenuChoice == 1) {
		cout << "Your Chose to Play";
		system("PAUSE");
		system("CLS");
	}
	else if (MainMenuChoice == 2) {
		cout << "You chose credits.";
		system("PAUSE");
		system("CLS");
	}
	else if (MainMenuChoice == 3) {
		cout << "You chose to exit.";
		system("PAUSE");
		system("CLS");
	}



	system("pause");
}
Last edited on
Devynao (30)

The primary reason for me using the while loop is there is a bit of user input error checking going on. It's not a complete error check but does a great deal of the checking. The user gets taken around until the selection fits the menu then returns 'from whence it came' with a feasible selection.


@kemort So how do I do it without the goto, but still keep the main menu displayed?
closed account (48T7M4Gy)
So how do I do it without the goto, but still keep the main menu displayed?


Please note the following points:
1. You will never have to use a goto whatever you are trying to do with your program.

2. System pause and cls confuse any programming issues you might be having. Students love them but they are not worth the trouble and are probably the reason you ask the question.

3. I have shown you how I would do it. If I want a new menu then I just add the relevant 'display_menu()' command where I want it.

4. Alternatively have a while loop that keeps running through the selection menu until the user selects exit. Like so, you'll have to write the actual code:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
 
while(haven't quit)
{
    display_Intro();
    display_Menu();
    menu_selection = get_menu_selection();
    
    if (menu_selection == 1) {
        cout << "Your Chose to Play";
    }
    else if (menu_selection == 2) {
        cout << "You chose credits.";
    }
    else
        {
             cout << "You chose to exit.";
             quit the program
         }
}


Last edited on
Devynao (30)
@kemort

I'm not trying to go against what your telling me, its just my head is geared a certain way and I want to try to do what I'm thinking.

Okay, so I definitely want to clear the screen somehow when the selection isn't 1, 2, or 3. Then return to the selection screen. This is what I have right now but if you select anything other than 123 it successfully clears the screen and displays it again but thinks I've selected 3.


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
 

#include <iostream>
#include <string>

using namespace std;

void display_Intro() {
	cout << "Welcome To Mist Light" << endl << endl << endl;
}

void display_Menu() {
	cout << "[1] Play     [2] Credits     [3] Exit" << endl << endl;
	
}

int get_menu_selection() {
	int menu_selection = 0;

cin >> menu_selection;
		
	if (menu_selection == 1) {
		cout << "Your Chose to Play";
	}
	else if (menu_selection == 2) {
		cout << "You chose credits.";
	}
	else if (menu_selection == 3) {
		cout << "You chose to exit.";
	}
	else {
		system("CLS");
			display_Menu();
		
			return menu_selection;
	}
}

void CharacterCreation() {

}

int main() {

	int menu_selection = 0;

	display_Intro();
	display_Menu();
	menu_selection = get_menu_selection();

	if (menu_selection == 1) {
		cout << "Your Chose to Play";
	}
	else if (menu_selection == 2) {
		cout << "You chose credits.";
	}
	else
		cout << "You chose to exit.";

	return 0;
}
Last edited on
Hengry (227)
In your main you have 
1
2
3
4
5
6
7
8
 
if (menu_selection == 1) {
		cout << "Your Chose to Play";
	}
	else if (menu_selection == 2) {
		cout << "You chose credits.";
	}
	else
		cout << "You chose to exit.";

and your get_menu_selection()function also contains a similar prompt for the user

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
 
 if (menu_selection == 1) {
		cout << "Your Chose to Play";
	}
	else if (menu_selection == 2) {
		cout << "You chose credits.";
	}
	else if (menu_selection == 3) {
		cout << "You chose to exit.";
	}
	else {
		system("CLS");
			display_Menu();
		
			return menu_selection;
	}

therefore, you're showing the prompt twice because in your main, you called the get_menu_selection() and then created a prompt when the user has selected an option. I would take out the prompt inside the main because the get_menu_selection() has error checking, so if the user enters anything that isn't 1, 2, or 3, it will display the menu again. The prompt inside main would exit even if the user's input is invalid.
Last edited on
Devynao (30)
@personxd1

I changed the else statement in my main from this :

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
 
int main() {

	int menu_selection = 0;

	display_Intro();
	display_Menu();
	menu_selection = get_menu_selection();

	if (menu_selection == 1) {
		cout << "Your Chose to Play";
	}
	else if (menu_selection == 2) {
		cout << "You chose credits.";
	}
	else
		cout << "You chose to exit.";

	return 0;
}


to this:
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
 
int main() {

	int menu_selection = 0;

	display_Intro();
	display_Menu();
	menu_selection = get_menu_selection();

	if (menu_selection == 1) {
		cout << "Your Chose to Play";
	}
	else if (menu_selection == 2) {
		cout << "You chose credits.";
	}
	else if (menu_selection == 3) {
		cout << "You chose to exit.";
	}
	else {
		system("CLS");
		display_Menu();
		menu_selection = get_menu_selection();
	}

	return 0;
}



and it works so far now after I changed that. If I take it out of my main it just closes if I dont enter 1, 2, or 3.
Last edited on
Devynao (30)
Well now if I enter the wrong number once, and enter it wrong a second time it closes.
closed account (48T7M4Gy)
I'm not trying to go against what your telling me

Don't feel as though there is only one way and/or you have to do it my way. The best way is to try things out. And the very best way is to write pseudocode before you write any code at all. Map out on paper exactly what you want to happen.

This following code is a suggestion and maybe you can adapt it to what you are trying to achieve. You might even be able to place the cls and pause lines strategically to avoid the scrolling screen - up to you. You can combine them into a new function and place the function where you want it. 

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

using namespace std;

void display_Intro() {
    cout << "Welcome To Mist Light" << endl << endl << endl;
}

void display_Menu() {
    cout << "[1] Play     [2] Credits     [3] Exit" << endl;
    cout << "    ^              ^              ^" << endl;
    cout << "     ^             ^             ^" << endl;
    cout << "      ^            ^            ^" << endl;
    cout << "       ^           ^           ^" << endl;
    cout << "        ^__________^__________^" << endl;
    cout << "                   ^" << endl;
    cout << "                   ^" << endl;
    cout << "                  ";
}

int get_menu_selection()
{
    int selection = 0;
    
    while(cout << "Pls enter a selection: " and cin >> selection and (selection < 1 or selection > 3) )
    {
        cout << "Error\n";
    }
    return selection;
}

void play()
{
    cout << "     >>>> Now we're PLAYing and running the 1000 line program that entails\n";
    return;
}

void credits()
{
    cout << "     >>>> Now we're doing whatever CREDITS is supposed to do\n";
    return;
}

int main() {
    
    int menu_selection = 0;
    
    display_Intro();
    while( menu_selection != 3 )
    {
        display_Menu();
        menu_selection = get_menu_selection();
        
        if (menu_selection == 1) {
            cout << "Your Chose to Play\n";
            play();
            cout << " *** I'm back at this place\n";
        }
        else if (menu_selection == 2) {
            cout << "You chose credits\n";
            credits();
            cout << " *** I'm back here\n";
        }
        else
            cout << "You chose to exit\n";
    }
    
    return 0;
}
Last edited on
Devynao (30)
@kemort
Don't feel as though there is only one way and/or you have to do it my way. The best way is to try things out. And the very best way is to write pseudocode before you write any code at all. Map out on paper exactly what you want to happen.

This following code is a suggestion and maybe you can adapt it to what you are trying to achieve. You might even be able to place the cls and pause lines strategically to avoid the scrolling screen - up to you. You can combine them into a new function and place the function where you want it.


This code makes more sense in my head. I like how once it finishes with lets say PLAY, it returns to the menu. The only thing that I would prefer is when you type in something that causes the error, is for it to tell you then press a key to clear the screen and return to the menu.
Pages: 12