2

I am trying to sort a map based on the value.

But I am seeing some weird behavior for the following program

public class CompareByValueMain {

    public static void main(String[] args) {
        Map<String,Integer> itemIDToTimeMap = new HashMap<String,Integer>();

        itemIDToTimeMap.put("Baggage", 3);
        itemIDToTimeMap.put("Handbag", 16);
        itemIDToTimeMap.put("Closed Footwear", 4);
        itemIDToTimeMap.put("Shirt", 25);

        Set<String> itemIDs = itemIDToTimeMap.entrySet().stream()
                .sorted(Collections.reverseOrder(Map.Entry.comparingByValue()))
                .map(Map.Entry::getKey)
                .collect(Collectors.toSet());

        System.out.println(itemIDs);
    }
}

The output is turned out to be correct

[Shirt, Handbag, Closed Footwear, Baggage]

But in the input when I change from Baggage to Bag It is giving the following output

[Shirt, Bag, Handbag, Closed Footwear]

Ideally sorting should be done based on the Value in the Map irrespective of the key values. But Not sure why it is changing if the key is changed here.

Pshemo
  • 122,468
  • 25
  • 185
  • 269
user2166328
  • 199
  • 2
  • 2
  • 6

3 Answers3

8

You are collecting the results into a Set. Not all Sets guarantee order.

So the sort works fine, but afterwards it is stored in a container that does not bother about the order of the Strings.

Use a container like List<> to store your data, after the sort has finished. This will guarantee the order of your items.

public class CompareByValueMain {

    public static void main(String[] args) {
        Map<String,Integer> itemIDToTimeMap = new HashMap<String,Integer>();

        itemIDToTimeMap.put("Bag", 3); // already changed to "Bag" to demonstrate the working code
        itemIDToTimeMap.put("Handbag", 16);
        itemIDToTimeMap.put("Closed Footwear", 4);
        itemIDToTimeMap.put("Shirt", 25);

        List<String> itemIDs = // use List<> instead of Set<>
                itemIDToTimeMap.entrySet().stream()
                .sorted(Collections.reverseOrder(Map.Entry.comparingByValue()))
                .map(Map.Entry::getKey)
                .collect(Collectors.toList()); // use .toList() instead of .toSet()

        System.out.println(itemIDs);
    }
}

A simple example to demonstrate the difference:

public static void main(String[] args) {
    System.out.println("List:");
    Stream
        .of("b", "a")
        .collect(Collectors.toList())
        .stream()
        .forEach(System.out::println);

    System.out.println();
    System.out.println("Set:");
    Stream
        .of("b", "a")
        .collect(Collectors.toSet())
        .stream()
        .forEach(System.out::println);
}

which outputs:

List:
b
a

Set:
a
b
slartidan
  • 20,403
  • 15
  • 83
  • 131
  • @slartidan For me the problem is the word `shuffle`. To me, that implies that the order is random, whereas your last block of code shows that it is not. A better way of putting it is that `Collectors.toSet` does not keep the original order. – Paul Boddington Nov 11 '15 at 21:59
6

Problem is here:

.collect(Collectors.toSet());

because Collectors.toSet() returns HashSet which is not ordered (since in most cases we don't really need an order in sets, but speed of its contains method which HashSet provides).

Use LinkedHashSet if you want to preserve insertion order

.collect(Collectors.toCollection(LinkedHashSet::new));

Or depending on how you want to use this result maybe use List instead?

Pshemo
  • 122,468
  • 25
  • 185
  • 269
4

The default toSet() collector returns an HashSet which does not retain the insertion order.

See the implementation in 8u60 (note that this is an internal detail):

public static <T> Collector<T, ?, Set<T>> toSet() {
    return new CollectorImpl<>((Supplier<Set<T>>) HashSet::new, Set::add,
                               (left, right) -> { left.addAll(right); return left; },
                               CH_UNORDERED_ID);
}

You can use .collect(Collectors.toCollection(LinkedHashSet::new)); to provide a specific implementation instead (which will retain the insertion order).

Alexis C.
  • 91,686
  • 21
  • 171
  • 177