HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaModerate

Payroll calculator

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
payrollcalculatorstackoverflow

Problem

First I tried to fetch this program as close to pseudo code where I must follow all imported items, all method definitions, and all defined variables; anything else was my choice.

After I created working program that best represent the sample run of output we were provided in my homework documentation, I decided to create checks for data input such as correct number input.

Now, is my code (sorry for expression) idiot-proof, meaning that whatever user inputs will not effect program's outputs?

What can I do to ensure that this code will not fail because of user input?

In my program I am trying to use while loops as much as possible to let user to re-enter data if data that was entered is not appropriate. But is this enough?

```
import java.io.*;

public class CalculatePayroll
{
public static final double TAX_SOCIAL = 0.10;
public static final double TAX_STATE = 0.10;
public static final double TAX_FEDERAL = 0.10;

public static void main(String[] args)
{
String inputName = null, mustBeNumeric = "[-+]?\\d+(\\.\\d+)?";
char inputStatus = 0, inputAskRepeat = 0;
double inputSalary = 0, inputRate = 0, inputHours = 0;
boolean boolRepeat = true;

BufferedReader input = new BufferedReader(new InputStreamReader(System.in));

System.out.println("Payroll Calculator");

while (boolRepeat == true)
{
boolean boolAskRepeat = true;
boolean boolAskStatus = true;

System.out.print("\nLast Name: ");

try {inputName = input.readLine(); }
catch (IOException e) { e.printStackTrace(); }

while (boolAskStatus == true)
{
System.out.print("Status (F or P): ");

try { inputStatus = input.readLine().toLowerCase().charAt(0); }
catch (IOException e) { e.printStackTrace(); }

if (inputStatus == "f".charAt(0))
{
boolAskStatus =

Solution

"User Proofing"

  • Don't use double



As already mentioned by toofarsideways in the comments, storing currency values as float or double is problematic. Floating point cannot represent all numbers; it will get you close though.

It is highly recommended that you use another option better suited for accuracy (like BigDecimal)

  • 0-length input



  • Optional last name



A user can skip entering a last name by pressing enter.

  • StringIndexOutOfBoundsException



A user that skips entering any other value will result in a thrown StringIndexOutOfBoundsException. I find myself wondering if it's necessary to only check the first character

Why not just compare Strings with equalsIgnoreCase(String)?

if("f".equalsIgnoreCase(inputStatus)) {
    // ... full-time
}


  • NullPointerException



readLine() can return null if the user presses CTRL+C. Typically CTRL+C will kill your program anyways, but still nice to avoid NullPointerExceptions.

  • Unchecked numeric values



A user can submit whatever numeric value they want for salary, rate, or hours without any special checks/sanitzation.

  • Negative values



A user can submit a negative salary, rate, or hours.

Instead of using regex to validate the input, I would rely on the built-in Double.parseDouble(String) method to deal with the validation. You only have to worry about handling the NumberFormatException.

System.out.print("Salary: ");
try { 
    stringCheckSalary = input.readLine(); 
    inputSalary = Double.parseDouble(stringCheckSalary);
    boolCheckSalary = false;
} catch (IOException e) {
    e.printStackTrace();
} catch(NumberFormatException ex) {
    System.out.printf("Please try again. Invalid number: '%s'\n", stringCheckSalary);
}


The same idea works for BigDecimal as well.

  • Unbounded numbers



You should validate the submitted values and make sure they make sense. Currently a user can input a salary of 700 trillion dollars, or maybe 400 hours of part time work in a week.

As an example to validate the number of hours, you might use

final int HOURS_IN_A_WEEK = 7 * 24; // should be static
boolCheckHours = (inputHours  HOURS_IN_A_WEEK);


  • Unclear error handling (usability)



When a user submits an unexpected value, you print the same request again without any indication of what went wrong.

You should print an error message detailing the problem before reasking for input.

For example,


Please try again. Invalid number: 'one hundred'

  • inputHours Bug



You accidentally parse checkRate as inputHours. So the rate and hours for part-time will always be the same.

inputHours = Double.parseDouble(stringCheckRate);


Other Code Comments

Since I'm at it I'll throw in a few comments on the code as well

  • roundDouble(double)



If you go with BigDecimal this will likely be a moot point.

roundDouble(double) is unecessarily complicated. In general you need to be careful when converting between double and int values. The loss of precision can cause some pretty serious problems in a real application.

If you're running Java 6+ then you can use DecimalFormat.setRoundingMode(RoundingMode),

DecimalFormat formatter = new DecimalFormat("0.00");
formatter.setRoundingMode(RoundingMode.DOWN);
System.out.println("-------------------------------------------------------");
System.out.println("Name\tStatus\t\tGross\tTaxes\tNet");
System.out.printf("%s\tPart Time\t%s\t%s\t%s\n\n",
        name,
        formatter.format(pay),
        formatter.format(calculateTaxes(pay)),
        formatter.format(pay - calculateTaxes(pay)));


If you're not running at least Java 6, you can simplify this method very easily as

private static double roundDouble(double number) {
    return Math.floor(number * 100.0) / 100.0;
}


Or if you're a fan of reusable methods,

private static double roundDown(double number, int precision) {
    // floorWithPrecision(2.617, 2) => Math.floor(2.617 * 100.0) / 100.0 => 2.61
    final double shiftValue = Math.pow(10, precision);
    return Math.floor(number * shiftValue) / shiftValue;
}


  • String.charAt(0)



Instead of using "f".charAt(0) to retrieve a single character you should use the char primitive 'f'

if (inputStatus == 'f')

Code Snippets

if("f".equalsIgnoreCase(inputStatus)) {
    // ... full-time
}
System.out.print("Salary: ");
try { 
    stringCheckSalary = input.readLine(); 
    inputSalary = Double.parseDouble(stringCheckSalary);
    boolCheckSalary = false;
} catch (IOException e) {
    e.printStackTrace();
} catch(NumberFormatException ex) {
    System.out.printf("Please try again. Invalid number: '%s'\n", stringCheckSalary);
}
final int HOURS_IN_A_WEEK = 7 * 24; // should be static
boolCheckHours = (inputHours < 0 || inputHours > HOURS_IN_A_WEEK);
inputHours = Double.parseDouble(stringCheckRate);
DecimalFormat formatter = new DecimalFormat("0.00");
formatter.setRoundingMode(RoundingMode.DOWN);
System.out.println("-------------------------------------------------------");
System.out.println("Name\tStatus\t\tGross\tTaxes\tNet");
System.out.printf("%s\tPart Time\t%s\t%s\t%s\n\n",
        name,
        formatter.format(pay),
        formatter.format(calculateTaxes(pay)),
        formatter.format(pay - calculateTaxes(pay)));

Context

StackExchange Code Review Q#15800, answer score: 11

Revisions (0)

No revisions yet.