79447029

Date: 2025-02-18 03:10:27
Score: 1
Natty:
Report link

Your comparator always returns 0

As @pebble unit points out in the first comment, using != for comparing your Integer objects is incorrect. It trickingly compares object references, not values. For demonstration I am comparing every entry in your map to every entry using a copy of your Comparator:

public static final Comparator<Map.Entry<String, Integer>> ENTRY_COMPARATOR = (a, b) -> {
    if (a.getValue() != b.getValue()) {
        return Integer.compare(b.getValue(), a.getValue());
    }
    return a.getKey().compareTo(b.getKey());
};

// ...

    for (Map.Entry<String, Integer> leftEntry : map.entrySet()) {
        for (Map.Entry<String, Integer> rightEntry : map.entrySet()) {
            System.out.println(leftEntry.getKey() + " " + rightEntry.getKey() + " " + leftEntry.getValue()
                    + " " + rightEntry.getValue() + " -> " + ENTRY_COMPARATOR.compare(leftEntry, rightEntry));
        }
    }

Output is:

accountB accountB 200 200 -> 0
accountB accountA 200 200 -> 0
accountB accountC 200 200 -> 0
accountA accountB 200 200 -> 0
accountA accountA 200 200 -> 0
accountA accountC 200 200 -> 0
accountC accountB 200 200 -> 0
accountC accountA 200 200 -> 0
accountC accountC 200 200 -> 0

Your value, 200, is outside the range where Java caches and reuses Integer objects (default up to 128, configurable). Therefore autoboxing gives you 3 different Integer objects all having the value 200 for your map. So != yields false and your comparator returns the result of Integer.compare(b.getValue(), a.getValue()). Which is 0 since the values are the same. So the entries are considered equal under your comparator.

What your priority queue does with elements that are considered equal is not specified. Which is why you get the elements in order that would appears to be random, yet reproducible.

The results I got

For documentation, from your code I got:

[accountB(200), accountC(200), accountA(200)]

I ran the code multiple times and always got this order.

The good comparator

The standard library has some nifty methods for building out comparators for us.

    PriorityQueue<Map.Entry<String, Integer>> maxHeap
            = new PriorityQueue<>(Map.Entry.<String, Integer>comparingByValue()
                    .reversed()
                    .thenComparing(Map.Entry::getKey));

Now the output is what I think you had desired:

[accountA(200), accountB(200), accountC(200)]

Map.Entry has methods comparingByKey and comparingByValue that offer comparators that compare keys and values, respectively. Since I read from your code that you want the values in descending order, I reversed() this comparator. Comparator.thenComparing is the standard way to chain comparisons so that if the first comparison yields equal, the next comparison is tried. You can of course chain as many comparisons as you can think of.

Reasons:
  • Long answer (-1):
  • Has code block (-0.5):
  • Unregistered user (0.5):
  • User mentioned (1): @pebble
  • Low reputation (1):
Posted by: Halim Mammadov