Correct my code: it seems far too complicated.

I wrote a program for beginning C++ class that calculates costs of long distance calls using "if" and "else". The assignment was as follows: "Write a program that prompts the user for the number of minutes the call lasted and outputs the amount due. The minutes will be an integer. The output should be formatted as follows with columns lined up and dollar amounts displayed with 2 decimal places. Calls under 3 minutes get charged the full $2.00 and a partial minute counts as a full minute for billing purposes".

The program works but I think it has some things in it that could be re-written to make it shorter. I'm getting a little tired and I think I'm missing ways to make it more concise. Some feedback would be great, 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
#include<iostream>
#include<iomanip>
using namespace std;
const float AnyCall= 3.99,AddMinCost=.45;
int main ()
{
	//Declare Variables 
	float  NumberOfMins,AddMins,TotalCost,AddCost;
	cout<<"Pease enter the number of minutes your call lasted: "<<endl;
	cin >>NumberOfMins;
        cout<<"\n";

	//Calculate number of minutes over three
	if (NumberOfMins>3)
	AddMins=NumberOfMins-3;
	else (NumberOfMins<3),AddMins=0;
	
	//Output call duration and standard charges
	cout<<"Call Duration: "<<ceil(NumberOfMins)<<" minutes\n\n";
	printf("Connection fee:%20c$ 1.99\n\n");
	printf("First 3 minutes:%19c$ 2.00\n\n");
	
	//Calculate additional minute charges
	cout<<ceil(AddMins)<<" additional minutes:"<<setfill(' ')<<right<<setw(16)<<"$ "
		<<fixed<<setprecision(2)<<ceil(AddMins)*AddMinCost,AddCost=ceil(AddMins)*AddMinCost;
	cout<<"\n"<<setfill('-')<<setw(42)<<"\n"<<setfill(' ')<<setw(15)<<"TOTAL:"
		<<setfill(' ')<<setw(22)<<"$ "<<AnyCall+AddCost<<endl;
	
	//Leave viewing window open
	cin.ignore();
	cin.get();
	return 0;
}
you not understanding the if-else structure. Your code should look 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<iostream>
#include<iomanip>
using namespace std;
const float AnyCall= 3.99,AddMinCost=.45;
int main ()
{
	//Declare Variables 
	float  NumberOfMins,AddMins,TotalCost,AddCost;
	cout<<"Pease enter the number of minutes your call lasted: "<<endl;
	cin >>NumberOfMins;
        cout<<"\n";

	//Calculate number of minutes over three
	if (NumberOfMins>3)
	     AddMins=NumberOfMins-3;
	else
             AddMins=0;
	
	//Output call duration and standard charges
	cout<<"Call Duration: "<<ceil(NumberOfMins)<<" minutes\n\n";
	printf("Connection fee:%20c$ 1.99\n\n");
	printf("First 3 minutes:%19c$ 2.00\n\n");
	
	//Calculate additional minute charges
	cout<<ceil(AddMins)<<" additional minutes:"<<setfill(' ')<<right<<setw(16)<<"$ "
		<<fixed<<setprecision(2)<<ceil(AddMins)*AddMinCost,AddCost=ceil(AddMins)*AddMinCost;
	cout<<"\n"<<setfill('-')<<setw(42)<<"\n"<<setfill(' ')<<setw(15)<<"TOTAL:"
		<<setfill(' ')<<setw(22)<<"$ "<<AnyCall+AddCost<<endl;
	
	//Leave viewing window open
	cin.ignore();
	cin.get();
	return 0;
}


I realize that the formatting systems of c are much cleaner that c++ but why are you mixing them since c++ has something equivalent.
also i hate to nag but you should get into the habit of using brackets with if statements if you ever deside to go to a programming competition the judges can be a little OCD and trust me ... you lose alot of points for improper syntax and how easy your code is read ive edited your code below

and whenever you use a constant the name should be in all capps im altering your code
advice programmer to programmer


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
#include<iostream>
#include<iomanip>
using namespace std;
const float ANYCALL= 3.99,ADDMINCOST=.45;
int main ()
{ 
	//Declare Variables 
	float  NumberOfMins,AddMins,TotalCost,AddCost;
	cout<<"Pease enter the number of minutes your call lasted: "<<endl;
	cin >>NumberOfMins;
        cout<<"\n";

	//Calculate number of minutes over three
	if (NumberOfMins>3)
        {
	     AddMins=NumberOfMins-3;
        }
	else
        {
             AddMins=0;
        }	
	//Output call duration and standard charges
	cout<<"Call Duration: "<<ceil(NumberOfMins)<<" minutes\n\n";
	printf("Connection fee:%20c$ 1.99\n\n");
	printf("First 3 minutes:%19c$ 2.00\n\n");
	
	//Calculate additional minute charges
	cout<<ceil(AddMins)<<" additional minutes:"<<setfill(' ')<<right<<setw(16)<<"$ "
		<<fixed<<setprecision(2)<<ceil(AddMins)*ADDMINCOST,AddCost=ceil(AddMins)*ADDMINCOST;
	cout<<"\n"<<setfill('-')<<setw(42)<<"\n"<<setfill(' ')<<setw(15)<<"TOTAL:"
		<<setfill(' ')<<setw(22)<<"$ "<<ANYCALL+AddCost<<endl;
	
	//Leave viewing window open
	cin.ignore();
	cin.get();
	return 0;
}



and btw good use of set precision
Last edited on
closed account (10oTURfi)
The reason to use brackets is to prevent ambiguous program.

1
2
3
4
if(something)
	if(something_else)
		if(something__)
		else do_something//To which if should this else apply? 
Last edited on
Thank you to everyone that commented, I don't mind the criticism at all. I jumped into this class with hardly any prior programming knowledge so my methods aren't all that great, I appreciate the help.
Topic archived. No new replies allowed.