I can't find what's wrong

#include <iostream>
using namespace std;

void welstudent()
{
cout << "Welcome to University Dear Students"<<endl;
}
int calfee(int nofc);
int calgpa(int gpa);
int section();
int main()
{
int fee,g,x;
cout << "Enter 1 to Claculate\nEnter 2 to Calculate GPA\nEner 3 to Allocate Section"<<endl;
cin>> x;
welstudent();

switch(x)
{
case 1:
{
calfee(fee);
break;
}
case 2:
{
calgpa(g);
break;
}

case 3:
{
section();
break;
}
}

int nofc,fee1=21000;
int calfee(int nofc)
{
cout << "Enter no of Courses = ";
cin >> nofc;
fee=fee1*nofc;
cout << "Fee of the course = "<<fee<<endl;
return fee;
}

char grade;
int gp1,gw=0,ch=3,tch=15;
int calgpa(int gpa)
{

for (int i=1;i<=5;i++)
{
cout << "Enter Grades of Courses = ";
cin >>grade;
if (grade=='a'||grade=='A')
gp1=4;
else if (grade=='b'||grade=='B')
gp1=3;
else if (grade=='c'||grade=='C')
gp1=2;
else if (grade=='d'||grade=='D')
gp1=1;
else if (grade=='f'||grade=='F')
gp1=0;
else
cout << "Invalid Charater!"<<endl;

gw=gw+gp1;
}

gpa=(gw*ch)/tch;
cout << "GPA of Courses = " << gpa <<endl;
return (gpa);
}

int rn,marks,rem,rev=0;
int section()
{
cout << "Enter You Roll No = ";
cin >> rn;
cout << "Enter Intermediate Marks = ";
cin marks;
n=marks;
rem=marks%10;
rev=(rev*10)+rem;
marks=marks/10;

if (n==rev)
cout << "Your Roll No "<<rn<< " is enrolled in Section BSSE-A "<<endl;
else if (n%2==0)
cout << "Your Roll No "<<rn<< " is enrolled in Section BSSE-B "<<endl;
else
cout << "Your Roll No "<<rn<< " is enrolled in Section BSSE-C "<<endl;

return rev;
}
Last edited on
Hello Usama911,

You could start with:


PLEASE ALWAYS USE CODE TAGS (the <> formatting button), to the right of this box, when posting code.

Along with the proper indenting it makes it easier to read your code and also easier to respond to your post.

http://www.cplusplus.com/articles/jEywvCM9/
http://www.cplusplus.com/articles/z13hAqkS/

Hint: You can edit your post, highlight your code and press the <> formatting button.
You can use the preview button at the bottom to see how it looks.

I found the second link to be the most help.


A few blank lines in your code would help to make it more readable.

Instead of asking what is wrong mention what it is not doing and copy and post any compile error messages that you receive. All this would help to narrow down what to look for.

It would also help if you mention what IDE/compiler you are using. Some people would find the version number helpful too.

You can work on that while I load up your program and see what happens.

Andy
Properly formatting your code would have made finding one of the problem(s) MUCH easier.

No closing brace in 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
#include <iostream>
using namespace std;

void welstudent()
{
   cout << "Welcome to University Dear Students" << endl;
}
int calfee(int nofc);
int calgpa(int gpa);
int section();
int main()
{
   int fee, g, x;
   cout << "Enter 1 to Claculate\nEnter 2 to Calculate GPA\nEner 3 to Allocate Section" << endl;
   cin >> x;
   welstudent();

   switch (x)
   {
   case 1:
   {
      calfee(fee);
      break;
   }
   case 2:
   {
      calgpa(g);
      break;
   }

   case 3:
   {
      section();
      break;
   }
   }

   int nofc, fee1 = 21000;
   int calfee(int nofc)
   {
      cout << "Enter no of Courses = ";
      cin >> nofc;
      fee = fee1 * nofc;
      cout << "Fee of the course = " << fee << endl;
      return fee;
   }

   char grade;
   int gp1, gw = 0, ch = 3, tch = 15;
   int calgpa(int gpa)
   {

      for (int i = 1; i <= 5; i++)
      {
         cout << "Enter Grades of Courses = ";
         cin >> grade;
         if (grade == 'a' || grade == 'A')
            gp1 = 4;
         else if (grade == 'b' || grade == 'B')
            gp1 = 3;
         else if (grade == 'c' || grade == 'C')
            gp1 = 2;
         else if (grade == 'd' || grade == 'D')
            gp1 = 1;
         else if (grade == 'f' || grade == 'F')
            gp1 = 0;
         else
            cout << "Invalid Charater!" << endl;

         gw = gw + gp1;
      }

      gpa = (gw * ch) / tch;
      cout << "GPA of Courses = " << gpa << endl;
      return (gpa);
   }

   int rn, marks, rem, rev = 0;
   int section()
   {
      cout << "Enter You Roll No = ";
      cin >> rn;
      cout << "Enter Intermediate Marks = ";
      cin marks;
      n = marks;
      rem = marks % 10;
      rev = (rev * 10) + rem;
      marks = marks / 10;

      if (n == rev)
         cout << "Your Roll No " << rn << " is enrolled in Section BSSE-A " << endl;
      else if (n % 2 == 0)
         cout << "Your Roll No " << rn << " is enrolled in Section BSSE-B " << endl;
      else
         cout << "Your Roll No " << rn << " is enrolled in Section BSSE-C " << endl;

      return rev;
   }


Using code tags would have made it easier for us to notice and tell you what one of the problems are.

Add the required closing brace and other problems -- declaring variables in wrong places, etc. -- will pop up.
Hello Usama911,

In addition to what Furry Guy has said.

Not having a closing brace ( } ) for "main" makes the functions that should follow "main" be part of main and that does not work.

It looks like it makes no difference where you have put the prototypes, but personally I like to put them before any actual functions. Also based on the way you have written your program I would put the "welstudent" function after "main" with the others.

When you get to "main" I have done this so far:
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
#include <iostream>

//using namespace std;  // <--- Best not to use.  // <--- Uncomment if you feel that you must use this.
// The most recent post that is worth reading. http://www.cplusplus.com/forum/beginner/258335/

//  Prototypes.
int calfee();
int calgpa();
int section();

void welstudent()
{
	std::cout << "\n      Welcome to University Dear Students" << std::endl;
}

int main()
{
	int fee{}, gpa{}, choice{};

	welstudent();

	std::cout << "\n 1 to Claculate"
		<< "\n 2 to Calculate GPA"
		<< "\n 3 to Allocate Section"
		<< "\n 4 to Exit"
		<< "\n   Enter selection: ";
	std::cin >> choice;


	switch (choice)
	{
		case 1:
			fee = calfee();
			break;

It is a good idea to initialize your variables when you define them. If only for the peace of mind that they do not contain garbage before they are used.

I also changes the names to something that makes more sense. It doe make the code easier to follow and understand.

So far I have only dealt with "case 1". In your original code you sent the variable "fee" to the function. At the function int calfee(int nofc) not only do you change the name of this variable it is for a different use, but it is only a copy of "fee" and any changes are lost when the function ends. Also the function returns an "int", (fee), that you never capture back in "main". Hard to understand what the point is of the return value since the function does everything that you need.

Also you wrote:
1
2
3
4
int nofc, fee1 = 21000;

int calfee(int nofc)
{

First you should try to avoid using global variables as it would allow anything that follows to be able to change their values and then it becomes hard to find where it went wrong.

Next defining "nofc" as a parameter makes this variable local to the function which overshadows the global variable and thus it would not receive any new value.

These variables are better defined inside the function where they are used.

What would work better is:
1
2
3
4
5
6
7
8
9
10
11
12
13
14
int calfee()
{
	constexpr int FEE1{ 21000 };
	int numOfCoursec{}, fee{};

	std::cout << "\n Enter no of Courses: ";
	std::cin >> numOfCoursec;

	fee = FEE1 * numOfCoursec;

	std::cout << "\n Fee of the course(s) is $" << fee << ".00" << std::endl;

	return fee;
}

To me "FEE1" feels like it should be a constant value that the function should never try to change.

After running the "calfee" function a few times I concluded the it would just as well this wayvoid calfee() taking no parameters and returning nothing. Since a returned value is never used in "main" at this time.

The same concepts would apply to the "calgpa" function.

A quick look at the "section" function looks OK for now although I have not tested that part yet.

Andy
Hello Usama911,

The beginning of the program now looks like 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
26
27
28
29
30
31
32
33
34
#include <cctype> // <--- std::tolower() and std::toupper(). http://www.cplusplus.com/reference/cctype/
#include <iostream>
#include <iomanip> // <--- http://www.cplusplus.com/reference/iomanip/

//using namespace std;  // <--- Best not to use.  // <--- Uncomment if you feel that you must use this.
// The most recent post that is worth reading. http://www.cplusplus.com/forum/beginner/258335/

void calfee();
void calgpa();
void section();
void welstudent();

int main()
{
	int choice{};  // <--- THese variables are not used in "main". fee{}, gpa{}

	welstudent();

	std::cout << "\n 1 to Claculate"
		<< "\n 2 to Calculate GPA"
		<< "\n 3 to Allocate Section"
		<< "\n 4 to Exit"
		<< "\n   Enter selection: ";
	std::cin >> choice;


	switch (choice)
	{
		case 1:
			calfee();
			break;
		case 2:
			calgpa();
			break;


The "calgpa" now starts like this:
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
void calgpa()
{
	constexpr int MAXCOURSES{ 5 };

	char grade{};
	double gpa{}, gp1{}, gw{ 0 }/*, ch = 3, tch = 15*/;

	std::cout << std::fixed << std::setprecision(4) << "\n Enter course grades. (A, B, C, D or F)\n" << std::endl;

	for (int i = 1; i <= MAXCOURSES; i++)
	{
		std::cout << " Enter Grades of Course " << i << ": ";
		std::cin >> grade;

		if (std::toupper(grade) == 'A')
			gp1 = 4;

The constant variable "MAXCOURSES" is a better choice than using a magic number in the for loop. Also easier to change if you need to.

Be careful when using "<=". This could produce one more loop than you may need. Especially when "i" starts at (0) zero and not (1).

I changed the "int"s to "double"s for the calculation of the "gpa". This is a more accurate representation of the "gpa" than using integer division which only deals with whole numbers.

I changed the prompt. To me it looks better this way. See what you think. It is still your choice.

The if statement is just an alternative to what you did. And there is nothing wrong with what you did.

The only part the I am confused about is gpa = (gw * ch) / tch;. Why multiply by (3) and then divide by (15) when this does the same: gpa = gw / MAZCOURSES;.

What I ended up with produces this output:


      Welcome to University Dear Students

 1 to Claculate  // <--- To calculate what?
 2 to Calculate GPA
 3 to Allocate Section
 4 to Exit
   Enter selection: 2

 Enter course grades. (A, B, C, D or F)

 Enter Grades of Course 1: a
 Enter Grades of Course 2: a
 Enter Grades of Course 3: z

    Invalid Charater!

 Enter Grades of Course 3: b
 Enter Grades of Course 4: a
 Enter Grades of Course 5: a

 GPA of Courses = 3.8000


 Press Enter to continue:


In the else statement you will need a set of {}s and some code to adjust the for loop and set "gp1" to (0)zero. Otherwise the for loop will continue with the next iteration and "gp1" will add the wrong number to "gw". Thus giving you the wrong "gpa".

In your last function when it asks for "Enter Your Roll No: " What number should be entered? Is this a number that came from the roll of a die or dice? Or just a number picked between 1 and 6? Also I think you may have meant "your" not "you".

And for "Enter Intermediate Marks: " I have no idea what should be entered. A test number or a range would be helpful.

Andy
Topic archived. No new replies allowed.