Payroll calculator

Question

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 = false;
 String stringCheckSalary = null;
 boolean boolCheckSalary = true;
 while (boolCheckSalary == true)
 {
 System.out.print("Salary: ");
 try { stringCheckSalary = input.readLine(); } 
 catch (IOException e) { e.printStackTrace(); }
 if (stringCheckSalary.matches(mustBeNumeric))
 {
 boolCheckSalary = false;
 inputSalary = Double.parseDouble(stringCheckSalary);
 }
 else 
 boolCheckSalary = true;
 }
 outputData(inputName, inputStatus, calculateFullTimePay(inputSalary));
 }
 else if (inputStatus == "p".charAt(0))
 {
 boolAskStatus = false;
 String stringCheckRate = null;
 boolean boolCheckRate = true;
 while (boolCheckRate == true)
 {
 System.out.print("Rate: ");
 try { stringCheckRate = input.readLine(); } 
 catch (IOException e) { e.printStackTrace(); }
 if (stringCheckRate.matches(mustBeNumeric))
 {
 boolCheckRate = false;
 inputRate = Double.parseDouble(stringCheckRate);
 }
 else 
 boolCheckRate = true;
 }
 String stringCheckHours = null;
 boolean boolCheckHours = true;
 while (boolCheckHours == true)
 {
 System.out.print("Hours: ");
 try { stringCheckHours = input.readLine(); } 
 catch (IOException e) { e.printStackTrace(); }
 if (stringCheckHours.matches(mustBeNumeric))
 {
 boolCheckHours = false;
 inputHours = Double.parseDouble(stringCheckRate);
 }
 else 
 boolCheckHours = true;
 } 
 outputData(inputName, inputStatus, calculatePartTimePay(inputRate, inputHours));
 }
 else boolAskStatus = true;
 }
 while (boolAskRepeat == true)
 {
 System.out.print("Another payroll? (Y or N): ");
 try { inputAskRepeat = input.readLine().toLowerCase().charAt(0); } 
 catch (IOException e) { e.printStackTrace(); }
 if (inputAskRepeat == "n".charAt(0))
 {
 boolAskRepeat = false;
 boolRepeat = false;
 }
 else if (inputAskRepeat == "y".charAt(0))
 {
 boolAskRepeat = false;
 boolRepeat = true;
 }
 else boolAskRepeat = true;
 }
 }
 System.out.print("\nProgram terminated...");
 }
 private static double calculateTaxes(double weeklyPay) 
 {
 return weeklyPay * (TAX_SOCIAL + TAX_STATE + TAX_FEDERAL);
 }
 private static double calculatePartTimePay(double rate, double hours) 
 {
 double pay = 0;
 if (hours <= 40) pay = rate * hours;
 else if (hours > 40) pay = (rate * 40) + ((hours - 40) * (rate * 1.5));
 return pay;
 }
 private static double calculateFullTimePay(double salary) 
 {
 return salary / 52;
 }
 private static void outputData(String name, char status, double pay) 
 {
 if (status == "p".charAt(0))
 {
 System.out.println("-------------------------------------------------------");
 System.out.println("Name\tStatus\t\tGross\tTaxes\tNet");
 System.out.printf("%s\tPart Time\t%.2f\t%.2f\t%.2f\n\n", 
 name, 
 roundDouble(pay), 
 roundDouble(calculateTaxes(pay)), 
 roundDouble(pay - calculateTaxes(pay)));
 }
 else if (status == "f".charAt(0))
 {
 System.out.println("-------------------------------------------------------");
 System.out.println("Name\tStatus\t\tGross\tTaxes\tNet");
 System.out.printf("%s\tFull Time\t%.2f\t%.2f\t%.2f\n\n", 
 name, 
 roundDouble(pay), 
 roundDouble(calculateTaxes(pay)), 
 roundDouble(pay - calculateTaxes(pay)));
 }
 }
 private static double roundDouble(double number) 
 {
 return Double.parseDouble(String.valueOf((int)(number * 100))) / 100;
 }
}

• toofarsideways: I'll put this as a comment as it's not really answering your question of idiot-proofness, but money shouldn't be represented in double or float, because they can't accurately represent most base 10 numbers. You should use either BigDecimal, int or long, with int and long representing pennies, cents or their equivalents. More info here -> javapractices.com/topic/TopicAction.do?Id=13
• smartcaveman: If you build an idiot proof program, they will build a better idiot.
• cl-r: "f" is a number in exadecimal. Silly doesn'it?
• HelpNeeder: The input statement which asks for character input is not idiot-proof, as I noticed. It cannot handle empty or too long string, but I have fixed that.

Answer 1

"User Proofing"

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

2. 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.

3. 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 < 0 || inputHours > HOURS_IN_A_WEEK);
     

4. 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'

5. 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

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

2. String.charAt(0)

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

   if (inputStatus == 'f')
   

• HelpNeeder: You have posten many things that I agree on. There was few things I coded there and I didn't agree too but part of this program was my homework. I just went further and tried to make this program as fail-proof as I could. I like bit of criticism on my coding :) Thank you!
• palacsint: readLine() returns null if the user presses CTRL+D (too). (It works on Linux.)
• Jared Paolin: @Palacsint I had a feeling about CTRL+D as well. My Linux box is down so I couldn't try it there. On windows it didn't seem to do anything especially noticeable.

Answer 2

Not related to the idiot-proof aspect of your code is the fact that it is structured badly, more precisely, it is not structured at all. At the very least your main function should be broken down into several smaller functions. Given that this is java, you should probably create one or more nested classes that handle input and/or validation.

Related to idiot proof, you should create a generic method of asking your question and verifying that the answer is acceptable, you do that and it will be idiot proof.

• HelpNeeder: I agree, the program, in fact, was not planned to be OOP so I just fetched it together and tried to make it fail safe :)

Answer 3

mustBeNumeric is a regular expression that will not change. It is better declared as a static final field, rather than a mutable variable within a method scope.

• tanyehzheng: Added for future reference: How to check a String is a numeric type in java

Answer 4

A few random notes:

1. I'd put the variable declarations to separate lines. From Code Complete, 2nd Edition, p759:

   With statements on their own lines, the code reads from top to bottom, instead of top to bottom and left to right. When you’re looking for a specific line of code, your eye should be able to follow the left margin of the code. It shouldn’t have to dip into each and every line just because a single line might contain two statements.

2. You could remove some duplication from the outputData method:

   private static void outputData(final String name, final char status, final double pay) {
    final String statusString;
    if (status == 'p') {
    statusString = "Part Time";
    } else if (status == 'f') {
    statusString = "Full Time";
    } else {
    throw new IllegalStateException("Invalid status: " + status);
    }
    System.out.println("-------------------------------------------------------");
    System.out.println("Name\tStatus\t\tGross\tTaxes\tNet");
    System.out.printf("%s\t%s\t%.2f\t%.2f\t%.2f\n\n", statusString, name, roundDouble(pay),
    roundDouble(calculateTaxes(pay)), roundDouble(pay - calculateTaxes(pay)));
   }
   

   I guess you could replace the similar if-else structures with polymorphism. Two useful reading:

   • Refactoring: Improving the Design of Existing Code by Martin Fowler: Replacing the Conditional Logic on Price Code with Polymorphism
   • Replace Conditional with Polymorphism

3. 52, 40 etc. should be named constant instead of magic numbers.

4. while (boolAskStatus == true)
   

   could be simply:

   while (boolAskStatus)
   

   From Code Complete, 2nd Edition, p433:

   Compare boolean values to true and false implicitly

   [...]

   Using implicit comparisons reduces the number of terms that someone reading your code has to keep in mind, and the resulting expressions read more like conversational English.