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

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;
}

Nov 10 '05 #1

John Harrison

br************@gmail.com wrote:

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.

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

Nov 10 '05 #2

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?

Nov 10 '05 #3

int2str

brian.digip...@gmail.com wrote:

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.
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>
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
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;
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 << ":";
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;
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;
}
Magic numbers - see above...
else if (NumExemptions == 1)
{
cout << setw(25);
cout << fixed;
cout << showpoint;
cout << setprecision(2);
cout << (GrossPay * .168);
FederalTax = GrossPay * .168;
}
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";
}

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

Nov 10 '05 #4

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.
:)

Nov 10 '05 #5

Neil Cerutti

On 2005-11-10, Brian <br************@gmail.com> wrote:

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.
:)

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.

--
Neil Cerutti

Nov 10 '05 #6

John Harrison

br************@gmail.com wrote:

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?

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

Nov 10 '05 #7

int2str

br************@gmail.com wrote:

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.

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

Nov 10 '05 #8

Ian

in*****@gmail.com wrote:

br************@gmail.com wrote:

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.

double gross = 0;
double federal_tax = 0;
double state_tax = 0;
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 ]
);

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

Nov 10 '05 #9