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 to 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?

Thank you in advance!

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

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')
   

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.

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.

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.