Getting Null in TreeMap using Comparator

Question

I have a class named Empl and two Comparators named as MySalaryComp and MyNameComp.

When I run this code I am getting null as a value in MySalaryComp as shown in output below.

public class Test{

    public static void main(String a[]) {
        TreeMap<Empl, String> tm = new TreeMap<Empl, String>(new MyNameComp());
        tm.put(new Empl("zzz", 3000), "RAM");
        tm.put(new Empl("aaa", 6000), "JOHN");
        Set<Empl> keys = tm.keySet();
        for (Empl key : keys) {
            System.out.println(key + " ==> " + tm.get(key));
        }

        TreeMap<Empl, String> trmap = new TreeMap<Empl, String>(new MySalaryComp());
        trmap.put(new Empl("zzz", 3000), "RAM");
        trmap.put(new Empl("aaa", 6000), "JOHN");
        Set<Empl> ks = trmap.keySet();
        for (Empl key : ks) {
            System.out.println(key + " ==> " + trmap.get(key));
        }
    }
}

class MyNameComp implements Comparator<Empl> {

    @Override
    public int compare(Empl e1, Empl e2) {
        return e1.getName().compareTo(e2.getName());
    }
}

class MySalaryComp implements Comparator<Empl> {

    @Override
    public int compare(Empl e1, Empl e2) {
        if (e1.getSalary() > e2.getSalary()) {
            return 1;
        } else {
            return -1;
        }
    }
}

class Empl {

    private String name;
    private int salary;

    public Empl(String n, int s) {
        this.name = n;
        this.salary = s;
    }

    public String getName() {
        return name;
    }

    public void setName(String name) {
        this.name = name;
    }

    public int getSalary() {
        return salary;
    }

    public void setSalary(int salary) {
        this.salary = salary;
    }


}

The output of above code is :

Name: aaa-- Salary: 6000 ==> JOHN
Name: zzz-- Salary: 3000 ==> RAM
Name: zzz-- Salary: 3000 ==> null
Name: aaa-- Salary: 6000 ==> null

Can some one help me understand why null values are showing ? And how to fix it.

Answer 1

Your comparator is not implemented correctly. Read up on the JavaDocs:

https://docs.oracle.com/javase/8/docs/api/java/util/Comparator.html

Compares its two arguments for order. Returns a negative integer, zero, or a positive integer as the first argument is less than, equal to, or greater than the second.

Try this:

@Override
public int compare(Employee e1, Employee e2) {
  return e1.getSalary() - e2.getSalary();
}

Why do you need to return 0?
If you look at the source code of TreeMap you will see this:

final Entry<K,V> getEntry(Object key) {
  // Offload comparator-based version for sake of performance
  if (comparator != null)
    return getEntryUsingComparator(key);
  if (key == null)
    throw new NullPointerException();
  @SuppressWarnings("unchecked")
  Comparable<? super K> k = (Comparable<? super K>) key;
  Entry<K,V> p = root;
  while (p != null) {
    int cmp = k.compareTo(p.key);
    if (cmp < 0)
      p = p.left;
    else if (cmp > 0)
      p = p.right;
    else
      return p; // <--Here
  }
  return null;
}

If the comparator is never 0 he will dereference the child branch which will be null.

Sidenote: You can also make your comparator functional, like so:

Comparator<Employee> salaryComparator = (e1, e2) -> (e1.getSalary() - e2.getSalary());
TreeMap<Employee, String> trmap = new TreeMap<>(salaryComparator);