Sorting map by size of value

Question

I just have a simple compare method to sort a Map by the size of value which is a set.

public List<Entry<String, HashSet<String>>> orderByDescStringSetSize(HashMap<String, HashSet<String>> map){
    Set<Entry<String, HashSet<String>>> set = map.entrySet();
    List<Entry<String, HashSet<String>>> list = new ArrayList<Entry<String, HashSet<String>>>(set);

    Collections.sort(list, new Comparator<Map.Entry<String, HashSet<String>>>(){
        public int compare(Map.Entry<String, HashSet<String>> o1, Map.Entry<String, HashSet<String>> o2){

            Integer o1Vals = o1.getValue().size();
            Integer o2Vals = o2.getValue().size();

            //descending
            if(o2Vals > o1Vals)
                return 1;
            else if(o2Vals==o1Vals)
                return 0;
            else
                return -1;

        }
    });
    return list;
}

I get java.lang.IllegalArgumentException: Comparison method violates its general contract! Why so?

Possible duplicate of ["Comparison method violates its general contract!"](https://stackoverflow.com/questions/8327514/comparison-method-violates-its-general-contract) — Guy, Aug 08 '17 at 07:58
Add an example of the Map that causes the exception to be thrown. — Oleg, Aug 08 '17 at 08:04
Yes on the duplication. I'd suggest a `return o2Vals - o1Vals;` and you should be fine. — daniu, Aug 08 '17 at 08:05
Thanks so much for all of you! I am sorry that I made a duplication. — Song, Aug 08 '17 at 08:27

score 2 · Accepted Answer · answered Aug 08 '17 at 08:16

2

It's because o2Vals==o1Vals is not doing what you expect.

You are working with Integer Objects - please use equals!!

This may not be the only issue.

answered Aug 08 '17 at 08:16

OldCurmudgeon

64,482
16
119
213

Thanks a lot!! I didn't think about that – Song Aug 08 '17 at 08:26
1

Or better, use `int` here, as there is no reason to use `Integer` when `size()` returns an `int`. Or simplify the entire method to `return Integer.compare(o1.getValue().size(), o2.getValue().size());` – Holger Aug 08 '17 at 15:06

score 0 · Answer 2 · answered Aug 08 '17 at 08:27

Using the same approach that you did I will modify:

The size of the HashSets is an int, use it for comparison rather than Integer.
No need to add if else if else if you are returning the value. Moreover, you can just return the values substracted: return o2Vals - o1Vals.
You are using Entry in some points and Map.Entry, are you sure you have the correct imports? In any case, I suggest you to use one or the other always to ensure the types are the same.

This said, the solution should look like:

public List<Entry<String, HashSet<String>>> orderByDescStringSetSize2(HashMap<String, HashSet<String>> map) {
    Set<Entry<String, HashSet<String>>> set = map.entrySet();
    List<Entry<String, HashSet<String>>> list = new ArrayList<>(set);

    Collections.sort(list, new Comparator<Entry<String, HashSet<String>>>() {

    @Override
    public int compare(Entry<String, HashSet<String>> o1, Entry<String, HashSet<String>> o2) {
        int o1Vals = o1.getValue().size();
        int o2Vals = o2.getValue().size();

        // Descending
        return o2Vals - o1Vals;
    }
    });
    return list;
}

However, if you are using Java 8 you can take advantage of the Stream interface and do it more compact without defining a Comparator:

public List<Entry<String, HashSet<String>>> orderByDescStringSetSize(HashMap<String, HashSet<String>> map) {
    return map.entrySet().stream() // Create stream
            .sorted(Comparator.comparing(entry -> -((Entry<String, HashSet<String>>) entry).getValue().size())) // Sort descending
            .collect(Collectors.toList()); // Collect
}

Sorting map by size of value

2 Answers2