0
\$\begingroup\$

How can I improve this?

#include "stdafx.h"
#include <iostream>

/*A program that accepts input of an employees' base salaries and years of service. 
Then it also calculates their bonus based on the years of service.
- 20 or more years of service, bonus = salary * 0.1 
- 10 or more years of service, bonus = salary * 0.05
- 5 or more years of service, bonus = salary * 0.025
Also the program should let the user to enter data until they want to stop. */

using namespace std;


int main() {
    int iSalary;
    int iService;
    float fBonus;
    int counter = 1;
    bool okcontinue = true;
    char idontchar; 

    while (okcontinue) { 
        cout << "You are employee #" << counter << " to use this.\n";
        cout << "Enter your base salary: \n";
        cin >> iSalary;
        cout << "Enter total # of years serviced: \n";
        cin >> iService;

        if (iService >= 20) fBonus = iSalary * 0.1;
        if ((iService < 20) && (iService >= 10)) fBonus = iSalary * 0.05;
        if ((iService < 10) && (iService >= 5)) fBonus = iSalary * 0.025;
        cout << "Your bonus is: " << fBonus << ". Enter 'e' to exit or 'c' to continue.\n";
        cin >> idontchar;

        if (idontchar == 'e') return 0;
        counter++;
        system("CLS");
    }
}
\$\endgroup\$
4
  • 3
    \$\begingroup\$ At the very least you could copy the formatting that was done for you over on stack overflow so somebody doesn't have to do it for you again. \$\endgroup\$ Commented Sep 9, 2011 at 22:40
  • 1
    \$\begingroup\$ Don't use Windows-specific stuff like #include "stdafx.h" and system("CLS"); unless you want your code to be totally non-portable i.e. limited to running under Windows only. \$\endgroup\$
    – Paul R
    Commented Sep 12, 2011 at 21:39
  • \$\begingroup\$ Read this stackoverflow question too stackoverflow.com/questions/1798511/… \$\endgroup\$
    – dex black
    Commented Sep 13, 2011 at 12:54
  • \$\begingroup\$ Read this stackoverflow question too for information about using an alternative to cin >> idontchar; stackoverflow.com/questions/1798511/… To remove a character that has been printed on the current line you can use putch('\b'); // backspace \$\endgroup\$
    – dex black
    Commented Sep 13, 2011 at 12:57

1 Answer 1

4
\$\begingroup\$
  1. Don't declare variables before you need them:

    int iSalary;
    int iService;
    float fBonus;
    int counter = 1;
    bool okcontinue = true;
    char idontchar; 
    

    By declaring them at the point of first use you also see that they are correctly initialized. Rather than having to scan all the way up the function to the top.

  2. Sure if this works for you:

    system("CLS");
    

    But it's not portable. Don't worry too much a bout clearing the screen (or learn to use a platform agnostic library like ncurses or equivalent).

  3. Tidy up your condition: (Nornagest nearly has it correct he needs to reverse the order and put in the else condition otherwise somebody with iService of >= 20 is going to be very happy with multiple bonus.)

    if      (iService >= 20) { fBonus = iSalary * 0.1;   }
    else if (iService >= 10) { fBonus = iSalary * 0.05;  }
    else if (iService >= 5)  { fBonus = iSalary * 0.025; }
    
  4. Floating point has rounding problems. Businesses do not like this. They would prefer things stay exact. So you may want to use integers instead of floats. But track the salary in pennies rather than dollars.

\$\endgroup\$
0

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.