Classes for acquiring employee information

I'm looking for a review on the following classes and the comments I applied (class assignment):

```
package assignment1;

/**
* Returns a GeneralEmployee object
* Sets the name, id for the object when called.
* It increments the id that is assigned to a new employee object so that each employee gets a unique id
* It can be used to compare if another GeneralEmployee object is equal
* @author me
*
*/
public abstract class GeneralEmployee implements Employee {

private String name;
private static int initialId = 0;
private int id;

/**
* Sets name to passed name param
* Increments initialID by 1
* sets id to value of intialId
* @param name
*/
public GeneralEmployee(String name) {
super();
this.setName(name);
initialId++;
id = Integer.valueOf(initialId);
}

/**
* Returns the name of the emloyee
* @return
*/
public String getName() {
return name;
}

/**
* Sets the name of the employee
* @param name
*/
public void setName(String name) {
this.name = name;
}

/**
* Returns the id of the employee
* @return
*/
public int getId() {
return id;
}

/*
* Returns a String comprising the name, id and pay of the employee
*/
public String toString() {
return "name " + getName() + " id " + getId() + " pay " + getPayCheckAmount();
}

/*
* Returns true if the GeneralEmployee object passed to it is equal to the current GeneralEmployee object
*/
public boolean equals(Object anOject){
GeneralEmployee generalEmployee;
try {
generalEmployee = (GeneralEmployee) anOject;
} catch (Exception e) {
return false;
}
return generalEmployee.id==id;
}

}

package assignment1;

/**
* This class extends GeneralEmployee
* It is used to set and get the hoursWorked and calculate payCheckAmount based

Just a couple of small points:

Constructor: setting fields

The general style of setting fields in a constructor looks like this:

this.name = name;

instead of this.setName(name);. Generally, it is good to use methods once they are defined, but in the case of a constructor, it is better to just set the field directly. If you use a setter, the setter might be overridden by a subclass, and then bugs could be introduced.

Integer valueOf integer

You don't need to call id = Integer.valueOf(initialId);. initialId is already an integer, so just do id = initialId.

@Override

If you override a method, add the @Override annotation, this makes your code easier to read. For example:

@Override
public String toString() {

equals

Use instanceof instead of a try-catch block, it will be faster.

Magic Numbers

Do not hardcode numbers like 40 or 1.5. Store it in a field instead, and give that field a good name. That way, if you need to change the value, you only need to change it in one place, and also the reader will know what eg 40 represents.

checkIfEmployeeIdIsDuplicate: deeply nested & bug

The checkIfEmployeeIdIsDuplicate method never returns true, this seems like a bug.

Also, the method is very deeply nested. To fix this, first of all use an early return like this:

if(employees == null){
    return true; // or throw exception
}

checkIfEmployeeIdIsDuplicate: unnecessary cast & another bug

Why store employees[i] as an Object, and then cast it to PartTimeEmployee? Just store it as PartTimeEmployee directly. Either way, if it is not a PartTimeEmployee, but a different kind of employee, this will cause an error.

Then, there wouldn't really be the need to store it, so your code could look like this:

private static boolean checkIfEmployeeIdIsDuplicate(Employee employees[]) {
    if (employees == null) {
        return true; // or throw exeption
    }
    for (int i = 0; i < employees.length; i++) {
        for (int j = 0; j < employees.length; j++) {
            if (employees[i] != null && employees[j] != null && i != j && employees[i].equals(employees[j])) {
                System.out.println(employees[i].toString() + " equals name " + employees[j].toString());
            }
        }
    }
    return false;
}

It still always returns false though (except if null is passed).

Naming

It is customary to use camelCase in Java, so for example DuplicateEmployeeId1 should be duplicateEmployeeId1. But it is not actually an id, so it should be duplicateEmployee1, or just employee1 (we don't even know yet if it is a duplicate).

StackExchange Code Review Q#61721, answer score: 4