Is there a better, more efficient way to write this program?

B

brian.digipimp

I turned this in for my programming fundamentals class for our second
exam. I am a c++ newb, this is my first class I've taken. I got a good
grade on this project I'm just wondering if there is a better and more
efficient way to write this program.

//This program calculates the users Gross Pay and subtracts the tax
based off of Marital Status
//and the number of exemptions they select

#include <iostream>
#include <iomanip>
#include <string>
#include <stdlib.h>
using namespace std;


int main()
{

string FirstName; //Holds the user inputed First Name
string LastName; //Holds the user inputed Last Name
char MaritalStatus; //Holds the user inputed value for Marital
Status
double HoursWorked; //Holds the user inputed value of Hours Worked
double HourlyRate; //Hodls the user inputed value of Hourly Rate
double GrossPay; //Calculates and holds the users inputed value for
Gross Pay
double FederalTax; //Holds the calculations for calculating
FederalTax
double StateTax; //Holds the calculations for calculating StateTax
int NumExemptions; //Holds the user inputed value for Number of
Exemptions


cout << "Enter your first name:";
cin >> FirstName;
cout << "Enter your last name:";
cin >> LastName;
cout << "Input the amount of hours worked:";
cin >> HoursWorked;
cout << "Input the hourly rate:";
cin >> HourlyRate;
cout << "Input the marital status ('M' for married and 'S' for
single): ";
cin >> MaritalStatus;
GrossPay = HoursWorked * HourlyRate;
StateTax = GrossPay * .0825;

switch (MaritalStatus)
{
case 'S':
cout << "Input the number of exemptions:";
cin >> NumExemptions;
cout << system("cls");
cout << "Payroll information for ";
cout << LastName << ", ";
cout << FirstName << ":";
cout << endl;
cout << endl;
cout << "Hours Worked: ";
cout << setw(24);
cout << fixed;
cout << showpoint;
cout << setprecision(2);
cout << HoursWorked;
cout << endl;
cout << "Hourly Rate: ";
cout << setw(25);
cout << fixed;
cout << showpoint;
cout << setprecision(2);
cout << HourlyRate;
cout << endl;
cout << "Marital Status: ";
cout << setw(22);
cout << "Single";
cout << endl;
cout << "Number of Exemptions: ";
cout << setw(16);
cout << NumExemptions;
cout << endl;
cout << endl;
cout << "Gross Pay";
cout << setw(29);
cout << fixed;
cout << showpoint;
cout << setprecision(2);
cout << GrossPay;
cout << endl;
cout << fixed;
cout << showpoint;
cout << setprecision(2);
cout << "Federal Taxes";


if (NumExemptions == 0)
{
cout << setw(25);
cout << (GrossPay * .179);
FederalTax = GrossPay * .179;
}

else if (NumExemptions == 1)
{
cout << setw(25);
cout << fixed;
cout << showpoint;
cout << setprecision(2);
cout << (GrossPay * .168);
FederalTax = GrossPay * .168;
}

else if (NumExemptions == 2)
{
cout << setw(25);
cout << fixed;
cout << showpoint;
cout << setprecision(2);
cout << (GrossPay * .157);
FederalTax = GrossPay * .157;
}

else if (NumExemptions == 3)
{
cout << setw(25);
cout << fixed;
cout << showpoint;
cout << setprecision(2);
cout << (GrossPay * .146);
FederalTax = GrossPay * .146;
}

else if (NumExemptions >= 4)
{
cout << setw(25);
cout << fixed;
cout << showpoint;
cout << setprecision(2);
cout << (GrossPay * .125);
FederalTax = GrossPay * .125;
}

cout << endl;
cout << "State Taxes";
cout << setw(27);
cout << fixed;
cout << showpoint;
cout << setprecision(2);
cout << GrossPay * .0825;
cout << endl;
cout << endl;
cout << "Net Pay";
cout << setw(31);
cout << (GrossPay - FederalTax - StateTax);
cout << endl;

break;

case 'M':
cout << "Input the number of exemptions:";
cin >> NumExemptions;
cout << system("cls");
cout << "Payroll information for ";
cout << LastName << ", ";
cout << FirstName << ":";
cout << endl;
cout << endl;
cout << "Hours Worked: ";
cout << setw(24);
cout << fixed;
cout << showpoint;
cout << setprecision(2);
cout << HoursWorked;
cout << endl;
cout << "Hourly Rate: ";
cout << setw(25);
cout << fixed;
cout << showpoint;
cout << setprecision(2);
cout << HourlyRate;
cout << endl;
cout << "Marital Status: ";
cout << setw(22);
cout << "Married";
cout << endl;
cout << "Number of Exemptions: ";
cout << setw(16);
cout << NumExemptions;
cout << endl;
cout << endl;
cout << "Gross Pay";
cout << setw(29);
cout << fixed;
cout << showpoint;
cout << setprecision(2);
cout << GrossPay;
cout << endl;
cout << fixed;
cout << showpoint;
cout << setprecision(2);
cout << "Federal Taxes";

if (NumExemptions == 0)
{
cout << setw(25);
cout << (GrossPay * .151);
FederalTax = GrossPay * .151;
}

else if (NumExemptions == 1)
{
cout << setw(25);
cout << fixed;
cout << showpoint;
cout << setprecision(2);
cout << (GrossPay * .142);
FederalTax = GrossPay * .142;
}

else if (NumExemptions == 2)
{
cout << setw(25);
cout << fixed;
cout << showpoint;
cout << setprecision(2);
cout << (GrossPay * .133);
FederalTax = GrossPay * .133;
}

else if (NumExemptions == 3)
{
cout << setw(25);
cout << fixed;
cout << showpoint;
cout << setprecision(2);
cout << (GrossPay * .124);
FederalTax = GrossPay * .124;
}

else if (NumExemptions >= 4)
{
cout << setw(25);
cout << fixed;
cout << showpoint;
cout << setprecision(2);
cout << (GrossPay * .105);
FederalTax = GrossPay * .105;
}

cout << endl;
cout << "State Taxes";
cout << setw(27);
cout << GrossPay * .0825;
cout << endl;
cout << endl;
cout << "Net Pay";
cout << setw(31);
cout << GrossPay - FederalTax - StateTax;
cout << endl;

break;

default: cout << "Invalid Status, Please type either 'M' for married
or 'S' for single";
}
return 0;
}
 
J

John Harrison

I turned this in for my programming fundamentals class for our second
exam. I am a c++ newb, this is my first class I've taken. I got a good
grade on this project I'm just wondering if there is a better and more
efficient way to write this program.
Click to expand...

Well define efficient, and explain why you are concerned about it.
Computers goes very fast you know, efficiency should be the absolute
least of your concerns. Right at the very, very bottom of the pile.

The single biggest improvement you could make would be to split the code
up into smaller functions instead of having everything in main. For
instance you could start with a process_single_person function and a
process_married_person function. This improvement has nothing to do with
efficiency, its about writing understandable, maintainable code. But
maybe you haven't covered writing functions yet.

john
 
B

brian.digipimp

Functions are actually in the next chapter. So the only thing I've done
with them was the prime numbers program last night. Im not really
concerned with the efficiency speed wise I guess, but code wise. Could
I have done it with less code just as well?
 
I

int2str

[email protected] said:
I turned this in for my programming fundamentals class for our second
exam. I am a c++ newb, this is my first class I've taken. I got a good
grade on this project I'm just wondering if there is a better and more
efficient way to write this program.
Click to expand...

More efficient is relative. If you mean coding and maintenacne time,
then yes!
See my comments below.
//This program calculates the users Gross Pay and subtracts the tax
based off of Marital Status
//and the number of exemptions they select

#include <iostream>
#include <iomanip>
#include <string>
#include <stdlib.h>
Click to expand...

What do you need stdlib.h for system() (system("cls") is not portable)?
Even if you needed it, wouldn't cstdlib be more appropriate?
using namespace std;


int main()
{

string FirstName; //Holds the user inputed First Name
Click to expand...

When your variable names are as expressive as they are in this case,
the comment is very redundant and not needed. Same for the rest of the
variable declarations.
string LastName; //Holds the user inputed Last Name
char MaritalStatus; //Holds the user inputed value for Marital
Status
double HoursWorked; //Holds the user inputed value of Hours Worked
double HourlyRate; //Hodls the user inputed value of Hourly Rate
"Holds"...

double GrossPay; //Calculates and holds the users inputed value for
Gross Pay
double FederalTax; //Holds the calculations for calculating
FederalTax
double StateTax; //Holds the calculations for calculating StateTax
int NumExemptions; //Holds the user inputed value for Number of
Exemptions


cout << "Enter your first name:";
cin >> FirstName;
cout << "Enter your last name:";
cin >> LastName;
cout << "Input the amount of hours worked:";
cin >> HoursWorked;
cout << "Input the hourly rate:";
cin >> HourlyRate;
cout << "Input the marital status ('M' for married and 'S' for
single): ";
cin >> MaritalStatus;

GrossPay = HoursWorked * HourlyRate;
StateTax = GrossPay * .0825;
Click to expand...

Magic number here. Use a constant instead to make this more expressive.

static const double StateTaxRate = 0.0825;
...
StateTax = GrossPay * StateTaxRate;
switch (MaritalStatus)
{
case 'S':
cout << "Input the number of exemptions:";
cin >> NumExemptions;
cout << system("cls");
cout << "Payroll information for ";
cout << LastName << ", ";
cout << FirstName << ":";
Click to expand...

This output code is very reduntant. You are doing this for both M and
S. So it would be much better to get it outside of the switch loop or
make it a a function. Use a ternary operator or similar where there are
slight differences between M and S.
cout << endl;
cout << endl;
cout << "Hours Worked: ";
cout << setw(24);
cout << fixed;
cout << showpoint;
cout << setprecision(2);
cout << HoursWorked;
cout << endl;
Click to expand...

Combine these into less lines:

cout << "Hours Worked: ";
cout << setw(24) << fixed << showpoint << setprecision(2) <<
HoursWorked << endl;

[more output code snipped...]
if (NumExemptions == 0)
{
cout << setw(25);
cout << (GrossPay * .179);
FederalTax = GrossPay * .179;
}
Click to expand...

Magic numbers - see above...
else if (NumExemptions == 1)
{
cout << setw(25);
cout << fixed;
cout << showpoint;
cout << setprecision(2);
cout << (GrossPay * .168);
FederalTax = GrossPay * .168;
}
Click to expand...

Once again you're repeating the output code many times. Why not simply
detemine the rate in a switch() and then output it at once. Kind of
like so (minus the magic numbers):

double FederalTaxRate = 0;

switch( NumExemptions )
{
case 0: FederalTaxRate = 0.179; break;
case 1: FederalTaxRate = 0.168; break;
case 2: FederalTaxRate = 0.157; break;
case 2: FederalTaxRate = 0.146; break;
default: FederalTaxRate = 0.125; break;
}

FederalTax = GrossPay * FederalTaxRate;

cout << setw(25) << fixed << showpoint << setprecision(2) <<
FederalTax << endl;

[mode eval code snipped]
default: cout << "Invalid Status, Please type either 'M' for married
or 'S' for single";
}
Click to expand...

It would be nicer to check for this at input time, so that one doesn't
have to re-enter all the other numbers just because he/she made a typo
at the marrital status.

Overall, you could easily trim 3/4 of your program and make it a lot
more readable without any loss of functionality.

As a general rule, everytime when you write the same code twice in a
row somewhere (or copy and paste a large block), you should ask
yourself "Why am I doing this?" and see if there's similarities between
the repeating blocks that you can streamline or combine into a
function.

Cheers,
Andre
 
B

Brian

I like those suggestions, thanks Andre. haha I like your comment about
my variable comments. I just copy and pasted all of my code, my
professor makes us comment every variable and explain them. If this
were done outside of my class I probably wouldn't have commented that.
:)
 
N

Neil Cerutti

I like those suggestions, thanks Andre. haha I like your comment about
my variable comments. I just copy and pasted all of my code, my
professor makes us comment every variable and explain them. If this
were done outside of my class I probably wouldn't have commented that.
:)
Click to expand...

It's a OK habit whilst you're learning, I guess.

If you're interested in good, professional programming style,
check out _The Practice of Programming_ by Kernighan & Pike. The
first chapter is full of good advice on naming your variables,
and the right way to use comments.
 
J

John Harrison

Functions are actually in the next chapter. So the only thing I've done
with them was the prime numbers program last night. Im not really
concerned with the efficiency speed wise I guess, but code wise. Could
I have done it with less code just as well?
Click to expand...

Well you could have chained your use of cout

For instance

cout << "Payroll information for " << LastName << ", " << FirstName <<
":" << endl << endl;
cout << "Hours Worked: " << setw(24) << fixed << showpoint <<
setprecision(2) << HoursWorked << endl;

instead of

cout << "Payroll information for ";
cout << LastName << ", ";
cout << FirstName << ":";
cout << endl;
cout << endl;
cout << "Hours Worked: ";
cout << setw(24);
cout << fixed;
cout << showpoint;
cout << setprecision(2);
cout << HoursWorked;
cout << endl;

There's a happy medium somewhere.

Don't think that less code is better code all the time however.
Sometimes splitting code up into small steps does aid understanding of
that code.

john
 
I

int2str

I turned this in for my programming fundamentals class for our second
exam. I am a c++ newb, this is my first class I've taken. I got a good
grade on this project I'm just wondering if there is a better and more
efficient way to write this program.
Click to expand...

Here's what I could come up with (yes, I'm bored :D).
Compiles and runs, didn't verify the calculations.

#include <iostream>
#include <iomanip>
#include <string>

using namespace std;

static const double state_tax_rate = 0.0825;

static const unsigned federal_table_max = 4;
static const double federal_rates_m[] = { 0.179, 0.168, 0.157, 0.146,
0.125 };
static const double federal_rates_s[] = { 0.151, 0.142, 0.133, 0.124,
0.105 };

int main()
{
string first_name, last_name;
char married = 'U';
double hours_worked = 0;
double hourly_rate = 0;
double gross = 0;
double federal_tax = 0;
double state_tax = 0;
unsigned exemptions = 0;

cout << "Enter your full name: ";
cin >> first_name >> last_name;

cout << "Hours worked: ";
cin >> hours_worked;

cout << "Hourly rate: ";
cin >> hourly_rate;

while( married != 'M'
&& married != 'S' )
{
cout << "Marrital status ('M' or 'S'): ";
cin >> married;
married = toupper( married );
}

cout << "Excemptions: ";
cin >> exemptions;

if ( exemptions > federal_table_max )
exemptions = federal_table_max;
gross = hours_worked * hourly_rate;
state_tax = gross * state_tax_rate;
federal_tax = gross * ( married == 'M' ?
federal_rates_m[ exemptions ] : federal_rates_s[ exemptions ]
);

cout << endl;
cout << fixed << showpoint << setprecision(2);

cout << "Payroll information for ";
cout << last_name << ", " << first_name << ":" << endl << endl;

cout << "Hours worked: " << hours_worked << endl;
cout << "Hourly wage: " << hourly_rate << endl;
cout << "Marrital status: " << ( married == 'M' ? "Married" :
"Single" ) << endl;
cout << "Exemptions: " << exemptions << endl << endl;

cout << "Gross pay: " << gross << endl;
cout << "Federal tax: " << federal_tax << endl;
cout << "State tax: " << state_tax << endl << endl;

cout << "Net pay: " << (gross - federal_tax - state_tax )
<< endl;
}

Notice the difference? ;)

Cheers,
Andre
 
I

Ian

double gross = 0;
double federal_tax = 0;
double state_tax = 0;
Click to expand...

Remove these three;
gross = hours_worked * hourly_rate;
state_tax = gross * state_tax_rate;
federal_tax = gross * ( married == 'M' ?
federal_rates_m[ exemptions ] : federal_rates_s[ exemptions ]
);
Click to expand...
make this:

double gross = hours_worked * hourly_rate;
double state_tax = gross * state_tax_rate;
double federal_tax = gross * ( married == 'M' ?
federal_rates_m[ exemptions ] : federal_rates_s[ exemptions ]);

trivial, I know :)

Ian
 

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments. After that, you can post your question and our members will help you out.

Ask a Question

Similar Threads

A few function questions 6
Is there a better way to do this? 8
Payroll Program using class structure 1
c++ code error - (working with vortexes) 4

Members online

No members online now.
Total: 104 (members: 0, guests: 104)
Robots: 459

Forum statistics

Threads
473,995
Messages
2,570,230
Members
46,818
Latest member
Brigette36

Latest Threads

Top