C++

Forum

 
If Else If vs Switch

Donut (8)
The code below works but I have to believe there is a better way. I am brand new to programming and could use some guidance on improving the code. I am using If Else If but the more I read about switches it seems like that may be a better option. Try not to laugh to hard, I am totally self taught.

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

using namespace std;

int main()
//Declare variables used in calculations
{
	double megabytes, totalB, totalE, totalP, package_B_cost, package_E_cost, package_P_cost;
	char package;
	bool Valid_Package = false;
	while(!Valid_Package){

//Display ISP Packages and include cost, MB included, and determine customer package
	cout << "We offer three ISP Packages, Basic (B), Enhanced (E), and Platinum (P).\n";
	cout << "Basic (B) is $35 and includes 5GB (5000 MB) and $.02/MB over the included 5GB.\n";
	cout << "Enhanced (E) is $75 and includes 10GB (10000 MB) and is $.01/MB over the included 10GB.\n";
	cout << "Platinum (P) is $120 and includes Unlimited Data\n";
	cout << "Which package do you have B, E, or P? : ";
	cin >> package;
	if(package == 'B' || package == 'b' || package == 'E' || package == 'e' || package == 'P' || package == 'p')
		Valid_Package = true;
      else
      cout << "Please enter one of the listed packages B, E, or P!! \n\n";
}
cout.setf(ios::fixed);
cout.setf(ios::showpoint);
cout.precision(2);
{
//Ask user for input
	bool Megabyte_Valid = false;
	while(!Megabyte_Valid){
	cout << "Enter how much data do you typically use: ";
	cin >> megabytes;
	if((megabytes >= 0) && (megabytes <= 100000))
		Megabyte_Valid = true;
	else
	   cout << "The acceptable range is between 0 and 100000. Please enter a valid amount. \n";
}

if((package == 'B') || (package == 'b'))
{
package_B_cost = 35;
package_E_cost = 75;
package_P_cost = 120;
if(megabytes < 5000)
{   
totalB = package_B_cost;
}
else
totalB = ((megabytes - 5000) * .02) + package_B_cost;
//Calculate charges based on user input of data used
cout << "The charges at the Basic Package rate are: $" << totalB << "\n\n";
}
if ((totalB<package_E_cost) && (package == 'B') || (package == 'b'))
   cout << "The Basic Package is a good choice for your usage.\n";
else if((totalB>package_E_cost) && (totalB<package_P_cost))
   cout << "You could save $ " << (totalB-package_E_cost) << " by switching to the Enhanced Package \n";
else if (totalB>120)
   cout << "You could save $ " << (totalB-package_P_cost) << " by switching to the Platinum Package \n";
else 
   ;
}
if((package == 'E') || (package == 'e'))
{
package_B_cost = 35;
package_E_cost = 75;
package_P_cost = 120;
if(megabytes < 10000)
totalE = package_E_cost;
else
totalE = ((megabytes - 10000) * .01) + package_E_cost;
//Calculate charges based on user input of data used
cout << "The charges at the Enhanced rate are: $" << totalE << "\n\n";
}
if ((totalE < package_E_cost) || (megabytes < 5000))
   cout << "You could save $ " << (totalE - package_B_cost) << " by switching to the Basic Package. \n"; 
else if (totalE < package_P_cost)
   cout << "The Enhanced Package is a good choice for your usage.\n";
else if (totalE > package_P_cost)
   cout << "You could save $ " << (totalE-package_P_cost) << " by switching to the Platinum Package. \n";
else
   ;

if((package == 'P') || (package == 'p'))
{
package_B_cost = 35;
package_E_cost = 75;
package_P_cost = 120;
totalP = package_P_cost;
cout << "The charges at the Platinum rate are: $" << totalP << "\n\n";
}
if ((megabytes < 5000) && (package == 'P' || package == 'p'))
   cout << "You could save $ " << (totalP-package_B_cost) << " by switching to the Basic Package. \n"; 
else if ((totalP>package_E_cost) && (totalP<package_P_cost) && (package == 'P' || package == 'p'))
   cout << "You could save $ " << (totalP-package_E_cost) << "by switching to the Enhanced Package.\n";
else if ((totalP<=package_P_cost) && (package == 'P' || package == 'p'))
   cout << "The Platinum Package is a good choice for your usage.\n";
else;
}
lastchance (6980)
@donut,

Your code is long and complicated for what it actually does - and therein, I suspect, lies the real problem. I don't have hard preferences either way between switch or a chain of if ... else if statements: I don't think that is a major problem.

Here are a few general suggestions.

(1) TBH, I was quite surprised when your code actually compiled. Your indentation is really incoherent. Try to lay the code out in a structured way. Blocks of code should be (consistently) indented. Where, for example, do the blocks starting on lines 28 and 31 terminate? - not where my eyes first told me that they did at any rate.

(2) There is a huge amount of repetition. Take, for example, lines 42-44, 65-67, 86-88. Do you really have to write the same thing 3 times? Refactor your code.

(3) You keep having to consider multiple choices to allow for the user entering an answer in either lower or upper case. But if you wrote
package = tolower( package );
immediately after reading it in then you would only have to consider the lower-case choice each time. See http://www.cplusplus.com/reference/cctype/tolower/

(4) Stroustrup's book has some sound advice - if you can't display a single function in a single text window ... then it is probably too long. You have a single function - int main() - and it is 95 lines long. Break your code up into functions. Also, some of the calculations look very similar, just with different parameters: a single function, called with different values of the parameters, might suffice.

(5) You have a lot of "magic numbers" - hard-coded specific values like 5000, 10000 - in hard-to-find segments of code. Suppose the ISP company decided to change a particular limit (mine does, far too regularly!) It would then have to scan through all the code, hopefully finding all values (and not touching others), but it would be very easy to make mistakes here. Any specific numbers like this should be assigned to informative variables, just once, somewhere near the start of the code. Then refer only to those variables later in the code.
Last edited on
Manga (1030)
The advise given by lastchance is both correct and wise, I also can't see where your program ends.
With int main() { you should end the program with return 0; I don't think I saw that anywhere.
Last edited on
jonnin (11331)
Ill try to address the specific question.

My personal style is this:
use if/else chains unless there is a very good reason to use a switch. Switches have a number of 'uglies' that I find are easier to just avoid outright --- they only work on integers, which is often a drawback, they effectively require defaults, and they have odd behavior if you forget to add a break statement, and nested/complicated decision logic gets very odd in switches.

So when to use a switch, then?
I will use them on very simple things, if for some insane reason I actually and to write an actual commandline program that had human interface and entry, I might control the menu with a simple switch instead of if/else pairs because it does look just a little cleaner. The other place I will use them is when you want to omit the breaks to fall through the cases in a way that simplifies the logic better than if/else pairs. This is rather rare in anything I have to do.

There are standard coding styles, and employers often have their own, and people just have opinions on what they like better. So the above is just how I do it, its just one opinion of many.

else ; is wasted. Either it gets optimized away by the compiler, or worse, it generates an actual no-op command and burns the cpu for nothing. And it bloats your code. The poor sap reading your code now has to determine if it is there for a reason because you are a genius or if its there for no reason for other, less polite monikers.

Don't take any of this as anything other than an attempt to help or be funny, though. You have a good program there, for a beginner who was self taught. Some of my early stuff was 1000 times worse. The best advice I can give you on style and such is just to read code from others, and this forum is a good place for that. See which programs look nice and cleanly solve the problem. Why do they look nice, why are they clean? Ask yourself how you can write like that next time. After a while, you will get there.

many programs will indent and clean up the formatting for you. I am not a visual person, and I don't even indent at all when I write, but let a tool do it afterwards. Indentation drives almost all programmers a bit nuts, though, I am an exception to that rule. Either way, just run it thru a tool, they are not perfect, but its good enough to save the sanity of your peers.


Last edited on
Donut (8)
Thanks for the responses and suggestions. It appears that when I stopped retyping my code and copied it into notepad++ I lost my indentations. I apologize.

I have spent a few hours trying to replace my "magic numbers" with variables because what lastchance said made sense. I will keep working my way through this and again I appreciate everyone's assistance.
Donut (8)
I worked on the code some today after reading through some tutorials and pondering on the posts above. Let me know what you guys think.
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
 
#include<iostream>

using namespace std;

int main()
//Declare variables used in calculations
{
	double  B_base_fee, B_data, B_Over_Rate, package_B_cost, E_base_fee, E_data, E_Over_Rate, package_E_cost, package_P_cost;
	char package;
	B_base_fee = 35;
	B_data = 5000;
	B_Over_Rate = .02;
	E_base_fee = 75;
	E_data = 10000;
	E_Over_Rate = .01;
	package_B_cost = 35;
	package_E_cost = 75;
	package_P_cost = 120;
	int data_used;
	int no_data = 0;
	int max_data = 100000;
	bool Valid_Package = false;
	while(!Valid_Package){

//Display ISP Packages and include cost, MB included, and determine customer package
	cout << "We offer three ISP Packages, Basic (B), Enhanced (E), and Platinum (P).\n";
	cout << "Basic (B) is $35 and includes 5GB (5000 MB) and $.02/MB over the included 5GB.\n";
	cout << "Enhanced (E) is $75 and includes 10GB (10000 MB) and is $.01/MB over the included 10GB.\n";
	cout << "Platinum (P) is $120 and includes Unlimited Data\n";
	cout << "Which package do you have B, E, or P? : ";
	cin >> package;
	if(package == 'B' || package == 'b' || package == 'E' || package == 'e' || package == 'P' || package == 'p')
		Valid_Package = true;
   else
      cout << "Please enter one of the listed packages B, E, or P!! \n\n";
   }
   cout.setf(ios::fixed);
   cout.setf(ios::showpoint);
   cout.precision(2);
   {
//Ask user for input
	bool Megabyte_Valid = false;
	while(!Megabyte_Valid)
   {
	cout << "Enter how much data do you typically use: ";
	cin >> data_used;
	if((data_used >= no_data) && (data_used <= max_data))
		Megabyte_Valid = true;
	else
	   cout << "The acceptable range is between 0 and 100000. Please enter a valid amount. \n";
   }
   switch(package)
   {
   case 'B':
   case 'b':
      if(data_used<B_data)
   {
      cout <<"The Basic Package Fee is $" <<B_base_fee <<endl;
   }else{
      package_B_cost = B_base_fee + ((data_used - B_data) * B_Over_Rate);
      cout <<"The Basic Package charges are: $" <<package_B_cost <<endl;
      if(package_B_cost>package_E_cost)
      cout <<"By switching to the Enhanced Package You could save $" <<package_B_cost - E_base_fee <<endl;
      if(package_B_cost>package_P_cost)
      cout <<"By switching to the Platinum Package You could save $" <<package_B_cost - package_P_cost <<endl;
        }
   break;
   case 'E':
   case 'e':
      if((B_data<data_used) && (data_used<=E_data))
   {
      cout <<"The Enhanced Package fee is $ "<<E_base_fee <<endl;
   }else{
      package_E_cost = E_base_fee + ((data_used - E_data) * E_Over_Rate);
      cout <<"The Enhanced Package charges are: $" <<package_E_cost <<endl;
      if(package_E_cost>package_P_cost)
      cout <<"By switching to the Platinum Package You could save $" <<package_E_cost - package_P_cost <<endl;
         }
   break;
   case 'P':
   case 'p':
   package_P_cost = 120;
   cout <<"The Platinum Package fee is $ "<<package_P_cost <<endl;
   if((package_P_cost>package_E_cost) && (data_used<=E_data))
   {
   cout <<"By switching to the Enhanced Package You could save $" <<package_P_cost - E_base_fee <<endl;
   }else{
   if((package_P_cost>package_B_cost) && (data_used<=B_data))
   cout <<"By switching to the Basic Package You could save $" <<package_P_cost - B_base_fee <<endl;
   }
  }
 return 0;
 }
}
Topic archived. No new replies allowed.