Is my code efficient?

I have an assignment that requires me to create a program which asks for a single letter grade (A,B,C,D,F) and based on that input, print a GPA (4,3,2,1,0). I wrote the program but I don't think it's the optimal solution. Can anyone give me tips on how to improve it? Thanks

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

#include <iostream>
using namespace std;

const int INVALID_GPA = -1;

int gpa(char letterGrade);

int main(){
	 char letterGrade;
	 cout << "Enter a letter grade: " << endl;
	 cin >> letterGrade;
     if(gpa(letterGrade) >= 0)
		  cout << "The GPA for that grade is: " << gpa(letterGrade) << endl;  
	 else
	     cout << "Invalid letter grade. Please try again." << endl;   
		 
   return 0;
}

int gpa(char letterGrade){

	
	  if(letterGrade == 'A' || letterGrade == 'a'){
		  return 4;
	  }
	  if(letterGrade == 'B' || letterGrade == 'b'){
		  return 3;
	  }
	  if(letterGrade == 'C' || letterGrade == 'c'){
		  return 2;
	  }
	  if(letterGrade == 'D' || letterGrade == 'd'){
		  return 1;
	  }
	  if (letterGrade == 'F' || letterGrade == 'f'){
		  return 0;
	  }
	  else 
		  return INVALID_GPA;
}


edit: I know the documentation and indentation is horrible, but please ignore that part as I will be fixing that soon.
Last edited on
Well, if you want to get nit picky, change that int to an unsigned int. Unsigned calculation and representation is easier for a computer. Also, you probably don't really need two calls to your function
well if u are not intending to input the marks and the grades to appear afterwards then i would say it is petty good.
You could use a switch statement instead of the if, if, if, if, if else statements and you could use tolower() or toupper() instead of letterGrade == 'C' || letterGrade == 'c' but its all just a matter of preference
if you use tolower() or toupper() make sure to #include <cctype>
Last edited on
If efficiency is your goal, I suggest you use a profiler (gprof is good and comes with GCC).
Alternatively, you can use the standard ctime functions to time the execution of each function, and then try something different. Look on Google if you can't find something interesting ;)
closed account (zb0S216C)
Here's a few small suggestions:

-Avoid global variables as much as possible
-Initialise all variables. Assignment isn't the same as initialisation
-When working with integers, I tend to favour switch()
-Declare a variable/parameter constant if their value doesn't change. The compiler can optimise constants
-Avoid implicit casts. If the right-hand side of an assignment doesn't match, the compiler will implicitly cast the right-hand side operand to the type that of the left-hand operand (if the types are close). Casts can be expensive

Wazzak
Last edited on
Topic archived. No new replies allowed.