Unexpected behavior when using Comparator.comparing(HashMap::get) as a comparator

Question

Doing the exercise 'Literature' on https://java-programming.mooc.fi/part-10/2-interface-comparable I discovered a very strange behavior when trying to sort key-value pairs in a HashMap, without copying anything to a TreeMap. I was supposed to add books, by making a Book class and adding them to a List. However I wanted to try without making a new class, so opted for the HashMap. My code was as follows:

public class MainProgram {

public static void main(String[] args) {
    Scanner scanner = new Scanner(System.in);

    Map<String, Integer> bookshelf = new HashMap<>();
    while (true) {


        System.out.println("Input the name of the book, empty stops: ");
        String bookName = scanner.nextLine();
        if (bookName.equals("")) {
            break;
        }
        System.out.println("Input the age recommendation: ");
        int age = Integer.valueOf(scanner.nextLine());

        bookshelf.put(bookName, age);
    }

    System.out.println(bookshelf.size() + " book" + (bookshelf.size() > 1 ? "s" : "") + " in total.");

    System.out.println("Books:");

    bookshelf.keySet().stream().sorted(Comparator.comparing(bookshelf::get)).forEach((key) -> System.out.println(key + " (recommended for " + bookshelf.get(key) + " year-olds or older)"));
}

}

using .sorted(Comparator.comparing(bookshelf::get)) was my idea of sorting them by the recommended age, which worked.

However, there exists an unexpected behavior that when the book's name is a single character ("A","b"), the program would also sort the keys alphabetically as though i made a comparator like Comparator.comparing(bookshelf::get).thenComparing(/*keys in keyset*/) but would sometimes also sort like aAbB

AA bb give unsorted results
AAA bbb give semi-sorted results in one or two buckets
AAAA bbbb give semi- or completely sorted results
AAAAA bbbbb and onward give unsorted results.

Can anybody explain what is happening here, at compiler level or somehow let me make sense of this?

Not the answer to your problem but why don't you write a book class and add a `compareTo()` method? Make benefit of object orientation! — elementzero23, Jun 07 '20 at 14:08
You sorted keyset based on the key only. If you want sort map use entryset() to avoid get call whle iterating. — Nomeswaran, Jun 07 '20 at 15:01

Jacob G. · Accepted Answer · 2020-06-04T17:53:06.657

9

bookshelf.keySet().stream().sorted(Comparator.comparing(bookshelf::get))

From the above snippet in your example, we can see that you're trying to sort the keys of bookshelf by their respective value.

The issue with this is that two book names could be mapped to the same age recommendation. Because you only have a single Comparator and because HashMap does not specify a consistent ordering, you have a chance at ending up with different results for the same inputs.

To ameliorate this, you can use thenComparing to handle the case when duplicate value-mappings are encountered:

bookshelf.entrySet()
         .stream()
         .sorted(Map.Entry.<String, Integer>comparingByValue().thenComparing(Map.Entry.comparingByKey()))
         .forEach(entry -> System.out.println(entry.getKey() + " (recommended for " + entry.getValue() + " year-olds or older)"));

edited Jun 04 '20 at 17:53

answered Jun 04 '20 at 17:28

Jacob G.

28,856
5
62
116

While I understand the correct method to do it, is there an explanation why it kinda-sorta works for n=1 and almost works for n=3 and n = 4? – Milos Cupara Jun 04 '20 at 17:42
3

@MilosCupara If you're referring to the length of the key as `n`, then it's just a coincidence that it *works* with those lengths. `HashMap` does not specify an ordering to its entries; but, in your case, the ordering is probably the same when the length of the keys are `1`, `3`, and `4`. However, if you continue adding more entries to the `Map` (more than `16`) whose keys are the same length, you'll see them be reordered. – Jacob G. Jun 04 '20 at 17:45
3

As a side note, `HashMap` does not only have an unpredictable iteration order, it also tells the stream via spliterator characteristics that it is not ordered, which allows the stream implementation to use an unstable sort algorithm when it considers it beneficial. So not even the particular iteration order perceived at a point of time is guaranteed to to be retained in the stream. Though, afaik, the current stream implementation always uses the same (stable) sort algorithm. – Holger Jun 05 '20 at 11:04

score 4 · Answer 2 · answered Jun 04 '20 at 17:25

4

This is happening since you are only using "key" to compare. You should compare them by both "key" and "value". This should work fine:

bookshelf.entrySet()
        .stream()
        .sorted(Map.Entry.<String,Integer>comparingByValue()
                .thenComparing(Map.Entry.comparingByKey()))
        .map(e -> e.getKey())
        .forEach((key) -> System.out.println(key + " (recommended for " + bookshelf.get(key) + " year-olds or older)"));

answered Jun 04 '20 at 17:25

Aniket Sahrawat

12,410
3
41
67

1

the `map` operation is redundant if you are planning to access the value within the `forEach` statement using `bookshelf.get(key)`. you could simplify to `.forEach(entry -> use entry.getKey and entry.getValue)` – Naman Jun 05 '20 at 03:48

0xh3xa · Answer 3 · 2020-06-04T17:52:15.953

4

Build the Comparator of Entry and use Entry::getValue and Entry::getKey to sort by value then by key

Comparator<Entry<String, Integer>> cmp = Comparator.comparing(Entry::getValue);

bookshelf.entrySet()
         .stream()
         .sorted(cmp.thenComparing(Entry::getKey))
         .forEach(entry -> System.out.println(entry.getKey() + " (recommended for " + entry.getValue() + " year-olds or older)"));

edited Jun 04 '20 at 17:52

answered Jun 04 '20 at 17:47

0xh3xa

4,801
2
14
28

Unexpected behavior when using Comparator.comparing(HashMap::get) as a comparator

3 Answers3