Malvinka
Malvinka

Reputation: 1379

Missing value in a TreeMap after putAll()

I have a HashMap that map character to an Integer. In order to sort it by value I wrote my comparator and I'm using TreeMap. But I am missing the value. I checked that for String "tree". My map 'chars' after for each loop looks like {r=1, t=1, e=2} and tree after putAll (two lines later) is {e=2, r=1}. What is happening to char 't'? Why is it missed? And how can I change it?

class ValueComparator implements Comparator<Character> {

private Map<Character, Integer> map;

public ValueComparator(Map<Character, Integer> map) {
    this.map = map;
}

public int compare(Character a, Character b) {
    return map.get(b).compareTo(map.get(a));
}
}

public String frequencySort(String s) {
    if (s.length() <= 1) return s;

    HashMap<Character,Integer> chars = new HashMap<Character,Integer>();
    for(Character c : s.toCharArray()){
        if (chars.containsKey(c)){
            chars.put(c,chars.get(c)+1);

        }
        else {
            chars.put(c,1);
        }  
    }

    TreeMap<Character,Integer> tree = new TreeMap<Character,Integer>(new ValueComparator(chars));
    tree.putAll(chars);

    /**
    * rest of the code
    **/

}

Upvotes: 0

Views: 754

Answers (1)

shmosel
shmosel

Reputation: 50716

Your ValueComparator treats entries with the same count as duplicates. A simple fix is to use the key as a tie-breaker:

public int compare(Character a, Character b) {
    int result = map.get(b).compareTo(map.get(a));
    return result != 0 ? result : a.compareTo(b);
}

Alternatively, you can use streams to build the frequency map, sort it and store it an ordered LinkedHashMap:

Map<Character, Integer> counts = s.chars()
        .mapToObj(i -> (char)i)
        .collect(Collectors.groupingBy(Function.identity(), Collectors.summingInt(c -> 1)))
        .entrySet()
        .stream()
        .sorted(Collections.reverseOrder(Entry.comparingByValue()))
        .collect(Collectors.toMap(Entry::getKey, Entry::getValue, (a, b) -> b, LinkedHashMap::new));

Upvotes: 2

Related Questions