Just want a little feedback

[SammyT79]

'Ello

I just completed my final individual assignment for my first java programming course. This is what I want to do and learning how to do it is what brought me to school. Unfortunately my class facilitator has decided that it is not in his best interest to contribute to my education.

My question is, would someone like to critique some code?

[SammyT79]

Here is the Employee object. 

/**
* This class holds employee names and values, and  
* calculates total commission, total compensation,
* and sales needed to meet the total compensation 
* earned by the highest earner.
*/

public class Employee
{
private String 
    firstName,
    lastName;
private double 
    salary, 
    salesTarget = 120000, //$120,000 sales target
    incentStart = 0.8, //Initiates incentive at 80% of salesTarget
    annualSales,
    differ = annualSales, //Protect integrity of annualSales variable
    annualSalesFinal,
    neededSales;

/**
* The constructor accepts string arguments that are assigned to 
* the firstName, laxtName, salary and annualSales fields.
* @param fName
* @param lName
* @param sal
* @param annSal
*/
public Employee(String fName, String lName, double sal, double annSal) 
{
    firstName = fName;
    lastName = lName;
    salary = sal;
    annualSales = annSal;
}

/**Returns the value stored in
* the firstName field
* @return 
*/
public String getFirstName()
{
    return firstName;
}

/**Returns the value stored in
* the lastName field
* @return 
*/
public String getLastName()
{
    return lastName;
}

/**Combines first and last names
* @return 
*/
public String getFullName()
{
    return firstName + " " + lastName;
}

/**Returns the value stored in
* the salary field
* @return 
*/
public double getSalary()
{
    return salary;
}

/**Sets the annualSales variable
* for use in the tables
* @param annSal
*/
public void setAnnualSales(double annSal)
{
    annualSales = annSal;
}

/**Returns the value stored in
* the annualSales field
* @return 
*/
public double getAnnualSales()
{
    return annualSales;
}    

/**Returns the value stored in
* the salesTarget field
* @return 
*/
public double getSalesTarget()
{
    return salesTarget;
}

/**Sets the incentStart variable
* for use in the tables
* @param start
*/   
public void setIncentStart(double start)
{
    incentStart = start;
}

/**Returns the value stored in
* the incentStart field
* @return 
*/
public double getIncentStart()
{
    return incentStart;
}

/**
* The getTotalCommis method calculates and returns the
* commission based on the applicable commission 
* and acceleration factor. Used only to display 
* total commission to the user.
* @return 
*/
public double getTotalCommis()
{
    double 
    commission, // Holds the commission
    accelFactor; // Holds the acceleration factor

if (annualSales >= salesTarget)       
{ 
    commission = 0.09;
    accelFactor = 1.25;
}  
else if (annualSales >= salesTarget * incentStart)
{ 
    commission = 0.09;
    accelFactor = 1;
}

else
{
    commission = 0;
    accelFactor = 1;
}

return (commission * annualSales) * accelFactor;
}

/**
* The getTotalComp method calculates and returns
* total compensation based on the applicable commission 
* and acceleration factor
* @return 
*/
public double getTotalComp()
{
    double 
    commission, // Holds the gross paycommission
    accelFactor; // Holds acceleration factor

if (annualSales >= salesTarget)       
{ 
    commission = 0.09;
    accelFactor = 1.25;
}  
else if (annualSales >= salesTarget * incentStart)
{ 
    commission = 0.09;
    accelFactor = 1;
}

else
{
    commission = 0;
    accelFactor = 1;
}

return ((commission * annualSales) * accelFactor) + salary;
}

/**
* The setNeededSales method calculates and returns
* the sales needed to meet the total compensation 
* earned by the highest earner.
*/
public void setNeededSales()
{
    double 
    commission, // Holds the gross paycommission
    accelFactor; // Holds acceleration factor

differ = differ + 1;

if (differ >= salesTarget)       
{ 
    commission = 0.09;
    accelFactor = 1.25;
}  
else if (differ >= salesTarget * incentStart)
{ 
    commission = 0.09;
    accelFactor = 1;
}

else
{
    commission = 0;
    accelFactor = 1;
}

neededSales = ((commission * differ) * accelFactor) + salary;
}

/**Returns the value stored in the neededSales field
* Used only to display the amount of sales needed to 
* meet the total compensation earned by the highest earner.
* @return 
*/
public double getNeededSales()
{
    return neededSales;
}

/**Returns the value stored in the differ field
* Used only to display the amount of sales needed to 
* meet the total compensation earned by the highest earner.
* @return 
*/
public double getDiffer()
{
    return differ;
}

/** Protects the integrity of annual sales input
*/
public void setAnnualSalesFinal()
{
    annualSalesFinal = annualSales;
}

/** Uses annualSalesFinal to reset annualSales
*/
public void resetAnnualSales()
{
    annualSales = annualSalesFinal;
}
}

[SammyT79]

and here is the rest. sorry 'bout the format. Had to delete a ton of white space to get it to work in here.

import java.text.DecimalFormat;
import java.util.Scanner;

public class CommissionCalc3
{
public static void main(String[] args)
{
// Initiate highest earner
String inputTest;
String highestEarner = null;
double highestTotalComp = 0;

// Create DecimalFormat objects to format dollar and percent output.
DecimalFormat dollar = new DecimalFormat("#,##0.00");
DecimalFormat percent = new DecimalFormat("#0%");

// Number of Employees
int NUM_EMPLOYEES; 

// Create a Scanner object for keyboard input.
Scanner keyboard = new Scanner(System.in);

System.out.println("How many employees are you evaluating?");

//***** test user input ********************************************************
boolean incorrectInput = true;
NUM_EMPLOYEES = -1;  

while (incorrectInput) 
{
if (keyboard.hasNextInt()) 
{
int n = keyboard.nextInt();
if (n < 2) 
{
System.err.println("This program is designed to "
+ "compare multiple employees.\n"
+ "Please enter a number larger than 1.");
} else 
{
NUM_EMPLOYEES = n;
incorrectInput = false;
}
} 
else 
{
System.err.println("Please enter a valid iteger larger than 1");
keyboard.next();
}
}
//***** end of input test ******************************************************

// Create an Employee array.
Employee[] user = new Employee[NUM_EMPLOYEES];

// Call the getEmployees method to get data for each employee.
getEmployees(user);

System.out.println("You entered the following:");

System.out.println();

// Display the data that the user entered.
for (int index = 0; index < user.length; index++)
{
System.out.println("Employee " + (index + 1));
System.out.println("Name: "
+ user[index].getFullName());
System.out.println("Salary: $"
+ dollar.format(user[index].getSalary()));
System.out.println("Annual sales: $"
+ dollar.format(user[index].getAnnualSales()));
System.out.println("Total commission: $"
+ dollar.format(user[index].getTotalCommis()));
System.out.println("Total compensation: $"
+ dollar.format(user[index].getTotalComp()));
System.out.println();

// Set highest earner and highest total compensation
if (user[index].getTotalComp() > highestTotalComp)
{
highestTotalComp = user[index].getTotalComp();
highestEarner = user[index].getFullName();
}
}

// Display highest earner and highest total compensation
System.out.println("Highest earner is " + highestEarner 
+ " with a total compensation of $" 
+ dollar.format(highestTotalComp));
System.out.println();

/** Initiate array to calculate sales needed to meet the total
* compensation of the highest earner.
*/
for (int index = 0; index < user.length; index++)
{
// Exclude highest earner
if (user[index].getTotalComp() != highestTotalComp)
{
// Continue testing data until total comp. reaches highest.
while (user[index].getNeededSales() < highestTotalComp)
{
// Increment sales +1 (refer to Employee object)
user[index].setNeededSales();

// Display needed sales when calculation ends.
if (user[index].getNeededSales() >= highestTotalComp)
{
System.out.println("In order for " 
+ user[index].getFullName() 
+ " to reach " + highestEarner 
+ "'s total compensation, sales would need to "
+ "be increased by $" 
+ dollar.format(user[index].getDiffer() 
- user[index].getAnnualSales()));
}
}
}
}

// Gives user the option to view projected income
System.out.println("\nWould you like to view projected income "
+ "for an employee? (Y or N) ");

String inputLine; // To hold a line of input
char choice; // To hold the user's choice

keyboard.nextLine(); // Consume the remaining newline.
inputLine = keyboard.nextLine(); // Store user input 
choice = inputLine.charAt(0); // Get the first char.

if (Character.toUpperCase(choice) == 'Y')
{
System.out.println("Please enter an employee number or -1 to quit.");

//***** test user input ********************************************************
incorrectInput = true;
int number = -1;  

while (incorrectInput) 
{
if (keyboard.hasNextInt()) 
{
int n = keyboard.nextInt();
if (n < -1 && n > NUM_EMPLOYEES -1) 
{
System.err.println("Pleas enter a valid employee number or -1 to quit");
} else 
{
number = n;
incorrectInput = false;
}
} 
else 
{
System.err.println("Please enter a valid iteger larger than 1");
keyboard.next();
}
}
//***** end of input test ******************************************************

keyboard.nextLine();

// Test for user input
while (number != -1)
{

//********* projected compensation table ***************************************                            
// Create a reset for annual sales
user[number - 1].setAnnualSalesFinal();

System.out.println();

System.out.println(user[number - 1].getFullName());

// Display the table headings
System.out.println("Total sales\t\t\t\tTotal compensation");
System.out.println("----------------------------------------"
+ "------------------");

// Declare controlExit variable 
double controlExit= user[number - 1].getAnnualSales() * 1.5;

// Innitiate loop for projected compensation table
while (controlExit >= user[number - 1].getAnnualSales())
{
if (user[number - 1].getAnnualSales() 
>= user[number - 1].getSalesTarget())
{
System.out.println("$" 
+ (dollar.format(user[number - 1].getAnnualSales()) 
+ "\t\t\t\t$" 
+ (dollar.format(user[number - 1].getTotalComp()))));

// increment annual sales
user[number - 1].setAnnualSales(user[number - 1].getAnnualSales() 
+ 5000);
}     
else if (user[number - 1].getAnnualSales() 
>= user[number - 1].getSalesTarget() 
* user[number - 1].getIncentStart()) 
{
System.out.println("$" 
+ (dollar.format(user[number - 1].getAnnualSales()) 
+ "\t\t\t\t$" 
+ (dollar.format(user[number - 1].getTotalComp()))));

// increment annual sales
user[number - 1].setAnnualSales(user[number - 1].getAnnualSales() 
+ 5000);

// Sales target benchmark declaration
if (user[number - 1].getAnnualSales() 
>= user[number - 1].getSalesTarget())
System.out.println("\nSales target hit, commission is "
+ "multiplied by set acceleration factor.");
}        
else  
{
System.out.println
("$" + (dollar.format(user[number - 1].getAnnualSales()) 
+ "\t\t\t\t$" 
+ (dollar.format(user[number - 1].getTotalComp()))));

// increment annual sales
user[number - 1].setAnnualSales
(user[number - 1].getAnnualSales() + 5000);

// Commission benchmark declaration
if (user[number - 1].getAnnualSales() 
>= user[number - 1].getSalesTarget() 
* user[number - 1].getIncentStart())
System.out.println("\nSales have reached " 
+ percent.format(user[number - 1].getIncentStart()) 
+ " of sales target. Commission is added to pay.");
}
}
// Reset annual sales to employee's annual sales
user[number - 1].resetAnnualSales();

// Prompt user to view another employee's 
// projected total compensation
System.out.println("Please enter an employee number or -1 to quit.");

number = keyboard.nextInt();
}         
}       
}
//* end of projected compensation table ****************************************

/**
* The getItems method accepts an Employee array as
* an argument. The user enters data for each element.
*/
private static void getEmployees(Employee[] array)
{
String firstName; 
String lastName; 
double salary;
double annualSales;

// Create a Scanner object for keyboard input.
Scanner keyboard = new Scanner(System.in);

// Prompt user to enter employee data.
System.out.println("Enter data for " + array.length
+ " employees.");

// Get data for the array.
for (int index = 0; index < array.length; index++)
{
// Get an employee first name.
System.out.println("\nEnter the first name of employee " 
+ (index + 1) + ": ");
firstName = keyboard.nextLine();

// Get an employee last name.
System.out.println("Enter the last name of employee " 
+ (index + 1) + ": ");
lastName = keyboard.nextLine();

// Get employee salary.
System.out.println("Enter the salary for employee " 
+ (index + 1) + ": ");

//********* test user input ****************************************************
boolean incorrectInput = true;
salary = -1;  

while (incorrectInput) 
{
if (keyboard.hasNextDouble()) 
{
double n = keyboard.nextDouble();
if (n < 0) 
{
System.err.println
("Please enter a number larger than 0");
} 
else 
{
salary = n;
incorrectInput = false;
}
} 
else 
{
System.err.println
("Please enter a valid number larger than 0");
keyboard.next();
}
}
//********* end of input test **************************************************

// Get employee sales.
System.out.println("Enter the annual sales for employee " 
+ (index + 1) + ": ");

//********* test user input ****************************************************
incorrectInput = true;
annualSales = -1; 

while (incorrectInput) 
{
if (keyboard.hasNextDouble()) 
{
double n = keyboard.nextDouble();
if (n < 0) 
{
System.err.println
("Please enter a number larger than 0");
} 
else 
{
annualSales = n;
incorrectInput = false;
}
} 
else 
{
System.err.println
("Please enter a valid number larger than 0");
keyboard.next();
}
}
//********* end of input test **************************************************

keyboard.nextLine(); // Consume the remaining newline.

// the data and store the object in the array.
array[index] = new Employee(firstName, lastName, salary, annualSales);
}       
}

private static boolean isValid(String test)
{
boolean goodSoFar = true; // Flag
char[] array; // To hold the input as an array

// Is the string the correct length?
if (test.length() > 8)
goodSoFar = false;

// Convert the string to a char array.
array = test.toCharArray();

// Analyze the characters.
for (int i = 0; i < array.length; i++)
{
if (!Character.isDigit(array[i]))
goodSoFar = false;
}

// Return the results
return goodSoFar;
}
}

[Traevel]

Regarding the JavaDoc, the @param and @return tags are not entirely filled with information. The convention is as follows:

/**
 * @param variableName Description that may contain multiple terms
 * @return A description that may contain multiple terms
 */

Also, it is common practice that the first sentence of the JavaDoc comment is a short sentence followed by a period.

/**
 * Short sentence that tells what this method does.
 * <p>
 * A longer description that can cover more than one
 * sentence and will provide extra information on the
 * method itself or usage of the method.
 */

Declaring the variables with comma's is possible but it doesn't help the reader who is interpretting your code. Also, when using this method to initialize mutable objects you can get unexpected behaviour as they would all refer to the same object instead of each receiving their own instance.

The bracket use is a choice. The only language I've ever heard it cause problems in is JavaScript where

bla bla {

}

would be preferred over

bla bla
{

}

But not in Java, so you can pick whichever one you like, just be consistent or it will be harder to read.

Not sure if it's your pasting in here or if it looks the same in your editor, but keep indentation consistent. So:

private void foo(){
    boolean bar = true;
    if(bar){
        // etc.
    }
}

instead of

private void foo(){
    boolean bar = true;
if(bar){
// etc.
}
}

You can use the format function of your IDE to get a file in the desired format very quickly, which can be set/altered using templates. Some of those are just a matter of preference, but it helps with the overall consistency.

On the second part:

int NUM_EMPLOYEES; suggests either a constant or a final variable. You are changing it on multiple occassions, so for clarity I wouldn't use all caps in this case.

You are testing edge cases, which is good. However, there are more than you are testing for. What would happen if I entered 999999999999999999999999999999999 or 1234 5679 (not saying these values will go wrong, just think further than 0 and null checks).

There is quite a bit of duplicate code in the checks for other numbers, try to the same with those as you did with isValid(String test) method (although I don't see where you are calling it). It will be easier to change and easier to read by others. There is such a thing as a "too complex" method.

Instead of using number - 1 in a million places because of the 0 based index (which you are correct in not showing the user) just do it once. It makes the code more readable.

Also, this check is looking strange to me if (n < -1 && n > NUM_EMPLOYEES -1). What you are saying is: if n is smaller than -1 AND larger than NUM_EMPLOYEES-1. Also, the use of && and || is usually meant to indicate that the second part will throw an error if the first condition fails or if the second condition would take a very long time to complete. Since that would not be the case here it would be clearer for the reader to stick with & and |.

A common example of this is to prevent the possible occurrence of a nullpointer. For instance:

if(myObject!=null && myObject.value()==5){
    // if myObject is null the && will make sure
    // the myObject.value() will not be called
}

if(myObject!=null & myObject.value()==5){
    // if myObject is null the & will allow the second
    // part to continue and it will cause a 
    // NullPointerException
}

What you did is not wrong, but by using the double && you are alerting the reader for something that is not happening.

Make sure System.err.println("Pleas enter a valid employee number or -1 to quit"); is correct, I don't see a test for number being -1 (could be because of the lack of indention and it's hard to count brackets to see what is inside the if's and what is outside) since you make -1 the default value for number.

On line 244-246 I don't see the same checks as before, if the user inputs a wrong employee number there, what would happen? Be sure to check that.

Lastly, you might want to close the scanner at the end of that long input session.

Keep in mind, when your teacher is testing, you might assume he won't take the time to enter a thousand employees. And he might not, but he could have a testing script ready that would easily be able to test that kind of amounts. He could also have a prepared list with all the edge cases so he won't miss one (and all students get tested the same).

Oh, I know you probably made that getEmployees() method static because the main is static and it wouldn't call a non-static getEmployees(). Keep an eye on that because if you follow that same tactic on more complex programs you'll end up with a ton of static methods that have no reason to be static. It's not bad practice to create an Object even if you know there will only ever be one of in your program. In fact, people have spent a lot of effort trying to find a way to ensure that only one instance could be made (a so called Singleton).

So just to be clear

public static void main(String[] args){
    Calculator myCalc = new Calculator();
    myCalc.getEmployees();
}

is an acceptable way to get to a non-static getEmployees().

All in all, not bad for a first Java course!

PS since a lot of the "conventions" are also a matter of opinion over time it's very well possible that others will see things differently. Be sure to remain open to their ideas as well, and stick with those you feel most comfortable with.

[JamesCherrill]

20 differ = annualSales,
What is this intended to do? annualSales wasn't initialised explicitly, so differ will just be a copy of its default value of 0

if (n < -1 && n > NUM_EMPLOYEES -1)
Unless NUM_EMPLOYEES is negative, this expression can never be true.

Lots of people (me included) don't have the time to decypher un-indented code.

ps: yes, conventions are matters of opinion, but for Java there are Sun/Oracle's own documented set of conventions that are always used within the Java development community (eg see the source code of the Java API classes), and so those are the ones you should use unless your employer/teacher requires otherwize.

[stultuske]

not to mention, it's pretty hard to decide whether code is good or not, if we don't know what it is supposed to do.

[SammyT79]

Traevel

For starters, thank you for your thorough response. It is much appreciated.

In response to your feedback:

Regarding the JavaDoc, declaring the variables with commas, and where to place brackets, I really had no Idea what was expected. I will fix it up now that I have a better idea. Thanks.

My indentation is much more consistent in the IDE. I just had a hard time posting it to the forum the way it was. I will try to figure out a better way to post next time.

On the second part:

I will fix NUM_EMPLOYEES, &&s, and ||s as suggested. I misunderstood their uses, thanks again. I will get rid of the million – 1s, not sure why I did it that way in the first place. As for “you might want to close the scanner at the end of that long input session” I had no idea.

One thing I would like to discuss. You mentioned my use of the isValid(String test). I called that method for salary and annual sales. I planned to utilize it for NUM_EMPLOYEES and to test the employee number that initiates the table, but I could not figure out how to test the value of the user input along with the isValid(String test) method. Here is one of my many failed attempts.

// Get number of employees
System.out.println("How many employees are you evaluating?");
    inputTest = keyboard.nextLine();

// Determine whether it is valid.
do
{
    if (!isValid(inputTest))
    {
        System.out.println("That is not a valid number.");
        System.err.println("Please enter a number.");
        inputTest = keyboard.nextLine();
    }
    if (isValid(inputTest))
    {
        int atLeastTwo = Integer.parseInt(inputTest);

        if (atLeastTwo < 2 )
        {
            System.out.println("This program is designed to "
                + "compare multiple employees.");
            System.err.println("Please enter a number larger than one.");
            atLeastTwo = keyboard.nextInt();
         }
     inputTest = Integer.toString(atLeastTwo);
     }     
}while (!isValid(inputTest));

// Convert to integer
NUM_EMPLOYEES = Integer.parseInt(inputTest);

I know the logic is not there. I had some earlier attempts that looked much nicer but I picked at it for days before I reverted back to what you saw in my original post.

[SammyT79]

JamesCherrill

I apologize for the un-indented code. Please bear with me as I am new to programming and and this is my first post to this, or pretty much any, forum.

In response to your comment - “20 differ = annualSales,
What is this intended to do? annualSales wasn't initialised explicitly, so differ will just be a copy of its default value of 0”

I created a table that calculated the employee’s projected annual compensation given sales increases of $5000 up to 150% of the employee’s annual sales. If I did not protect the integrity of the original annual sales input, a second table for the same employee would be inaccurate. I probably didn’t do it right, but I’m kind of just “shooting from the hip” here.

[SammyT79]

stultuske

Here is what was required.
My first task:
Write a Java™ application using NetBeans™ Integrated Development Environment (IDE) that calculates the total annual compensation of a salesperson. Consider the following factors:
A salesperson will earn a fixed salary of <Add a salary figure here>.
A salesperson will also receive a commission as a sales incentive. Commission is a percentage of the salesperson’s annual sales. The current commission is <Add a percentage here> of total sales.
The total annual compensation is the fixed salary plus the commission earned.
The Java™ application should meet these technical requirements:
The application should have at least one class, in addition to the application’s controlling class (a controlling class is where the main function resides).
There should be proper documentation in the source code.
The application should ask the user to enter annual sales, and it should display the total annual compensation.

My second task:
Modify the Week Two Java™ application using Java™ NetBeans™ IDE to meet these additional and changed business requirements:
The company has recently changed its total annual compensation policy to improve sales.
A salesperson will continue to earn a fixed salary of <Add salary figure from Week Two here>. The current sales target for every salesperson is < Add a target sales figure here (e.g. $120,000)>.
The sales incentive will only start when 80% of the sales target is met. The current commission is <Add percentage here> of total sales.
If a salesperson exceeds the sales target, the commission will increase based on an acceleration factor. The acceleration factor is <Add a number greater than 1 (e.g. 1.25)>.
The application should ask the user to enter annual sales, and it should display the total annual compensation.
The application should also display a table of potential total annual compensation that the salesperson could have earned, in $5000 increments above the salesperson’s annual sales, until it reaches 50% above the salesperson’s annual sales.
The Java™ application should also meet these technical requirements:
The application should have at least one class, in addition to the application’s controlling class.
The source code must demonstrate the use of conditional and looping structures.
There should be proper documentation in the source code.

My final task “what I posted”
Modify the Week Three Java™ application using Java™ NetBeans™ IDE to meet these additional and changed business requirements:
The application will now compare the total annual compensation of at least two salespersons.
It will calculate the additional amount of sales that each salesperson must achieve to match or exceed the higher of the two earners.
The application should ask for the name of each salesperson being compared.
The Java™ application should also meet these technical requirements:
The application should have at least one class, in addition to the application’s controlling class.
The source code must demonstrate the use of Array or ArrayList.
There should be proper documentation in the source code.

[Traevel]

but I could not figure out how to test the value of the user input along with the isValid(String test) method

Why not use two? A number and a String would require different tests anyway. I would keep the scanner outside of it though, less confusing that way.

Something like:

String userinput;

// example with Strings

do{
    userinput = keyboard.nextLine();

    // do stuff here

}while(!isValidString(userinput));


// stuff inbetween


// example with numbers

do{
    userinput = keyboard.nextLine();

    // do stuff here

}while(!isValidNumber(userinput));

With validation methods something like:

private boolean isValidNumber(final String test){
    try{
        int number = Integer.parseInt(test);

        if(number < 2){

            // perhaps a printline here if you want to
            // inform the user, although a more proper way
            // would be to throw a custom exception holding
            // the error message. That way this method will
            // be even more "standalone" and could be used
            // by all sorts of classes to validate numbers
            // who can deal with the exceptions in their
            // own ways

            return false;
        }
    }catch(NumberFormatException e){

        // same thing about exceptions here

        return false;
    }

    // more tests here

    // all tests were passed so it must be valid
    return true;
}

private boolean isValidString(final String test){

    if(test == null | "".equals(test)){

        // again, same thing about exceptions

        return false;
    }

    // more tests here

    // all tests passed so it's valid
    return true;
}

In my opinion it's a good habit to get used to doing the (slightly unnatural feeling) "".equals(string) instead of string.equals(""). The first one will not throw a NullPointerException if string is null (after all, "" does not equal null so it will just return false). Because of that we can use the |, no risk there.

Since the input for isValidNumber(final String test) is a String you could test for string validity as well to prevent some duplication, like so:

private boolean isValidNumber(final String test){

    // check string validity first
    if(!isValidString(test)){

        // error printing/throwing etc.

        return false;
    }

    // rest of method

    return true;
}

Just be aware that in that case all tests in isValidString(final String test) would also be applied to your numeric inputs.

Regarding your sample, NUM_EMPLOYEES = Integer.parseInt(inputTest);, just keep in mind that conversions and casts can (almost) always throw some sort of exception. Read the method's JavaDoc

// public static int parseInt(String s)
//                   throws NumberFormatException

// *snip*

// Throws:
//   NumberFormatException - if the string does not contain a parsable integer.

to figure out which one(s). You should always catch the Exceptions mentioned there.

[JamesCherrill]

I have to disagree with Traevel about && vs &
Firstly:
The JLS 15.23 says: "&& computes the same result as & on boolean operands. It differs only in that the right-hand operand expression is evaluated conditionally rather than always."

So it depends on if it matters whether the RHS is evaluated or not. Sometimes it's essential that it isn't (eg x!=null && x.something). More rarely it's essential that it is (eg RHS calls a mthod with essential side-effects like lazy initialisation of something, or may need to throw an Exception to reveal an error).

In the most usual case, where it doesn't matter if the RHS is evaluated unneccessarily, the && version has a slight efficiency advantage, that's all.
For that reason most people automatically use && rather than & unless there's a need for the RHS always to be evaluated.

This preference is so deep that when the Oracle tutorials introduce logical operators they don't even mention the & version ( http://docs.oracle.com/javase/tutorial/java/nutsandbolts/op2.html )

(And ditto for || vs |)

ps: if(x!=null && x.something)... is a common idiom that saves a couple of keystrokes compared to the explicit meaning of if (x!=null) if (x.something) ...
This usually happens when null is used as a valid value for a reference, indicating the absence of a real instance. Now Google "Tony Hoare and the Billion Dollar Mistake".
That's why Java 8 has added the java.util.Optional<T> class that, when used as intended, will eliminate the vast majority of those null checks.

[Traevel]

The way you describe its usage would suggest that a reader seeing & expects something uncommon to happen in the right hand part. Whereas I always believed that seeing && would trigger that expectation.

It's a good reason to switch from default & to default && since I must admit I can't remember the last time I absolutely required that the second part would be tested, regardless of the first result.

The slight speed efficiency is something I found less important than readability. For tests where speed would begin to be a factor, in my former way of reasoning, I would have used && (and thus have warned the reader that a more resource intensive test was about to follow).

In summary, something about age, dogs and tricks.

[JamesCherrill]

" If a question can be framed at all, it is also possible to answer it. ~ Ludwig Wittgenstein"

Didn't Godel disprove that?

[Traevel]

Yes he did. I just like his boldness in tractatus, claiming he had solved all philosophical problems. This is how it is, period, no need to waste more time on the subject. I'm not a big philosophy fan in my spare time, but that's one of the exceptions I didn't mind reading.

[SammyT79]

I don't know what is going on here anymore but, thanks to your help, I got the input tests to work perfectly... exceptions and all.

[SammyT79]

It also looks much better!

[JamesCherrill]

Yes, sorry. We did hijack your thread for a side discussion about & and &&. That was a bit naughty of us.
Anyway, I'm really glad that you got it working properly. Well done.

[Traevel]

Ditto, and well done indeed.