Why HashSet calling compareTo method and causing NullPointerException

Question

I am adding values in HashSet and getting NullPointerException in compareTo method.

java.lang.NullPointerException
    at com.fiveIQ.document.Link.compareTo(Link.java:226)
    at com.fiveIQ.document.Link.compareTo(Link.java:16)
    at java.util.HashMap.compareComparables(HashMap.java:371)
    at java.util.HashMap$TreeNode.treeify(HashMap.java:1920)
    at java.util.HashMap.treeifyBin(HashMap.java:771)
    at java.util.HashMap.putVal(HashMap.java:643)
    at java.util.HashMap.put(HashMap.java:611)
    at java.util.HashSet.add(HashSet.java:219)
    at com.fiveIQ.crawlData.parser.EightyLegJsonParser.parse(EightyLegJsonParser.java:51)
    at com.fiveIQ.crawlData.processor.CrawlDataParser.process(CrawlDataParser.java:57)
    at com.fiveIQ.crawlData.processor.CrawlDataUploader.upload(CrawlDataUploader.java:31)
    at com.fiveIQ.crawlData.processor.CrawlDataUploaderExecutor$1.run(CrawlDataUploaderExecutor.java:85)
    at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142)
    at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617)
    at java.lang.Thread.run(Thread.java:745)

Here is my compareTo method. Values of updatedOn is null. But I am not getting why HashSet is calling compareTo?

@Override
public int compareTo(Link o)
{
    return o.updatedOn.compareTo(this.getUpdatedOn());
}

Answer 1

I suppose you are using Java 8. Since Java 8 the HashMap implementation (which is used by HashSet) is changed to protect against collisions: if the same bucket has too many elements and keys are Comparable, then this bucket is converted to the R-B tree (similar to TreeMap) to make search in that bucket logarithmic instead of linear. This also protects application from malicious attempts to poison HashMaps with colliding keys causing denial-of-service attacks. See JEP 180 for details.

Answer 2

Under certain circumstances newer versions of HashMap (from Java 8 onwards) try to use a tree as the bin for objects with the same hash code instead of a linked list. It only does it if your objects implement Comparable, and it has every right to do so as your objects advertise themselves as being Comparable.

The lesson is: if you implement Comparable in your object, your compareTo() method must fulfill its contract, specifically they should only fail with a NullPointerException if the argument passed to them is null. In your case one of the Link objects has updatedOn set to null, this causes the problem. What you should have instead is something like this:

@Override
public int compareTo(Link o)
{
    if (o.updatedOn == null) {
        return this.updatedOn == null ? 0 : 1; //or -1, depending on whether you want null to come first or last in the ordering
    ]
    return o.updatedOn.compareTo(this.updatedOn); // it isn't a good idea to use getter for one object and direct field access for the other
}

This implementation won't fail with a NPE but it still has an issue: what happens if two different links are updated on the same date? What happens is that sometimes they will be treated as equal. At other times they won't. Your code will fail in subtle and unpredictable ways.

The reason for that is that updatedOn doesn't contain enough information about Link objects to uniquely identify them, it shouldn't be used in compareTo() on its own.

So the second lesson is: be careful to only implement Comparable if your objects have a "natural" order that can be derived from their properties. In other words: only objects that can be treated as identical if neither is greater than the other be Comparable.