A little help here please!!!

Hello guys.I have made a program that asks the user to type his/her name and password.It will continue only if username is "user" and password is "pass".Otherwise it will ask again for the correct details.After he/she login he/she will have a menu and some submenus to choose.My problem is that all the code is in main.There are no functions.Should I create some in order to simplify the main?

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

using namespace std;
main()
{

int pick,pick2,pick3;
string name;
string pass,pick4;




cout<<"Hello user.Please login to your account\n";
start1:
cout<<"Type your username\n";
cin>>name;
cout<<"Now your password\n";
cin>>pass;

do {


if ((name=="user")&&(pass=="pass"))
{
cout<<"Welcome "<<name<<"\n";
}

else {
cout<<"Wrong details\n";
goto start1;
}

}
while (name!="user" and pass!="pass");




cout<<"My name is IF4T and I am your personal computer.\nNow you have some options to choose\n";
start3:
cout<<"1. My Documents\n2. My Account\n3. Media Center\n4.Language\n5.Quit\n";
cin>>pick;

do switch (pick){

	case 1: cout<<"There are no documents.\n";
		cout<<"Do you want to go back?[Yes/No]\n";
	start4:
	cin>>pick4;
	if (pick4=="Yes"||"yes"||"YES"){
		goto start3;
	}
else if (pick4=="No"||"NO"||"no"){
	goto end1;
}
else {
cout<<"I dont understand.Please select Yes or No]\n";
goto start4;
}break;
	case 2: cout<<"No pictures\n";
		cout<<"Do you want to go back?[Yes/No]\n";
	start5:
	cin>>pick4;
	if (pick4=="Yes"||"yes"||"YES"){
		goto start3;
	}
else if (pick4=="No"||"NO"||"no"){
	goto end1;
}
else {
cout<<"I dont understand.Please select Yes or No]\n";
goto start5;
}break;
	case 3: cout<<"1. Video\n2. Music\n3. Pictures\n";
start:                                 

	cin>>pick2;
	
	if (pick2==1)
{
		cout<<"No video files\n";
}
		
		else if (pick2==2){
			cout<<"No Music files\n";
			
		}
		else if (pick2==3){
			cout<<"No pictures\n";
		}
		
		
	else
	{cout<<"Please select again.\n";
goto start;
	}
	cout<<"Do you want to go back?[Yes/No]\n";
	start6:
	cin>>pick4;
	if (pick4=="Yes"||"yes"||"YES"){
		goto start3;
	}
else if (pick4=="No"||"NO"||"no"){
goto end1;}
else {
cout<<"I dont understand.Please select Yes or No]\n";
goto start6;
}
	break;
		
	case 4: cout<<"1. English\n2. German\n3. Frence\n"; 
	start2:	
	cin>>pick3;
	
	if (pick3==1){
		cout<<"English\n";
}
		
		else if (pick3==2){
			cout<<"German\n";
			
		}
		else if (pick2==3){
			cout<<"Frence\n";
		}
	else
	{cout<<"Please select again.\n";
goto start2;
	}	cout<<"Do you want to go back?[Yes/No]\n";
	start7:
	cin>>pick4;
	if (pick4=="Yes"||"yes"||"YES"){
		goto start3;
	}
else if (pick4=="No"||"NO"||"no"){
	goto end1;
}
else {
cout<<"I dont understand.Please select Yes or No]\n";
goto start7;
}break;
	case 5:cout<<"quiting...\n";exit;break;
	default:cout<<"Invalid choice"<<endl;}
	while (!(pick>=1 and pick<=5));
end1:
system ("pause");

}
Hi,

Yes you should definitely use functions. There is a convention that functions should be less than 40 LOC, including main().

main() returns an int by the way, so it should be int main() {

While you are at it, you could get rid of the goto , use loops instead. This is not a hard and fast rule for everyone, but it is for beginners :+)

Also, this :

139
140
141
142
143
144
	if (pick4=="Yes"||"yes"||"YES"){
		goto start3;
	}
else if (pick4=="No"||"NO"||"no"){
	goto end1;
}


If you restrict the input to y or n, the logic is much simpler. One can use toupper or tolower to get a consistent case.

If you were to continue using this, then it would have to look like this:

if (pick4=="Yes" || pick4 == "yes" || pick4== "YES")

But you haven't accounted for every combination, so that is why I recommend using a single char as input.

Good Luck ! :+)


Edit:

I try to avoid do loops - it often means that end conditions are tested twice, and they can be error prone. Were you aware that all 3 types of loops can be converted to one of the other types. So I prefer while loops when the exact number of iterations isn't known (some condition instead), and for loops when the number of iterations is known.

Just because a loop must be executed once, doesn't necessarily mean that it has to be a do loop.
Last edited on
Thanks for your reply.I will try to fix it.
When validating input, I often find it easiest to break out of the loop in the middle. For example:
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
    cout<<"Hello user.Please login to your account\n";
    while (true) {
	cout<<"Type your username\n";
	cin>>name;
	cout<<"Now your password\n";
	cin>>pass;

	if ((name=="user")&&(pass=="pass")) {
	    break;
	} else {
	    cout<<"Wrong details\n";
	}
    }

    cout<<"Welcome "<<name<<"\n";

Topic archived. No new replies allowed.