The analyzer has detected a situation where the 'compareTo' method of a parent class, which implements the 'Comparable<T>' interface, is overloaded in the child class. However, it is very unlikely that the programmer really meant to overload the parent class's method.
This is an example of a class that does method overloading rather than overriding:
public class Human implements Comparable<Human>
{
private String mName;
private int mAge;
....
Human(String name, int age)
{
mName = name;
mAge = age;
}
....
public int compareTo(Human human)
{
int result = this.mName.compareTo(human.mName);
if (result == 0)
{
result = Integer.compare(this.mAge, human.mAge);
}
return result;
}
}
public class Employee extends Human
{
int mSalary;
....
public Employee(String name, int age, int salary) {
super(name, age);
mSalary = salary;
}
....
public int compareTo(Employee employee)
{
return Integer.compare(this.mSalary, employee.mSalary);
}
}
So, we have two classes: the base class 'Human' and derived class 'Employee'. 'Human' implements the 'Comparable<Human>' interface and defines the 'compareTo' method. The derived class 'Employee' extends the base class and overloads the 'compareTo' method. The comparison method returns one of the following results:
The implications may be as follows:
1) If we create 'Employee' objects using a reference to the base class and then call to the 'compareTo' method, the objects will not be compared properly:
Human emp1 = new Employee("Andrew", 25, 33000);
Human emp2 = new Employee("Madeline", 29, 31000);
System.out.println(emp1.compareTo(emp2));
What will be printed is the value -12, which suggests that the emp1 object is logically less than emp2. But that is not so. The programmer must have really intended to compare the objects' 'mSalary' fields, which would produce just an opposite result. This bug occurred because the programmer had overloaded the comparison method rather than overriding it, so when it was called, it was called from the 'Human' class.
2) As you know, lists of elements implementing the 'Comparable<T>' interface can be automatically sorted using 'Collections.sort'/'Arrays.sort', and such elements can be used as keys in sorted collections, without the need to specify a comparator. With such method overloading, the sort will be performed in a way different from what the programmer intended and defined in the derived class. The dangerous thing about this is that in such cases, the comparison method is called implicitly, which makes the bug very difficult to find.
Let's run the following code:
List<Human> listEmployees = new ArrayList<>();
listEmployees.add(new Employee("Andrew", 25, 33000));
listEmployees.add(new Employee("Madeline", 29, 31000));
listEmployees.add(new Employee("Hailey", 45, 55000));
System.out.println("Before: ");
listEmployees.forEach(System.out::println);
Collections.sort(listEmployees);
System.out.println("After: ");
listEmployees.forEach(System.out::println);
The program will print the following:
Before:
Name: Andrew; Age: 25; Salary: 33000
Name: Madeline; Age: 29; Salary: 31000
Name: Hailey; Age: 45; Salary: 55000
After:
Name: Andrew; Age: 25; Salary: 33000
Name: Hailey; Age: 45; Salary: 55000
Name: Madeline; Age: 29; Salary: 31000
As you can see, the list is sorted by a different field, not 'mSalary'. The reason is just the same.
To fix this problem, we need to make sure that the comparison method is overridden, not overloaded:
public class Employee extends Human
{
....
public int compareTo(Human employee)
{
if (employee instanceof Employee)
{
return Integer.compare(this.mSalary,
((Employee)employee).mSalary);
}
return -1;
}
....
}
The code will now work just as expected.
In the first case, the program will produce the value 1 (emp1 is logically greater than emp2).
In the second case, it will print the following:
Name: Andrew; Age: 25; Salary: 33000
Name: Madeline; Age: 29; Salary: 31000
Name: Hailey; Age: 45; Salary: 55000
After:
Name: Madeline; Age: 29; Salary: 31000
Name: Andrew; Age: 25; Salary: 33000
Name: Hailey; Age: 45; Salary: 55000