0

I can not figure out why my custom UpdateableTreeMap class is not working. It is supposed to sort a TreeMap by its value.

Full code here:

import org.bukkit.entity.Player;

import java.util.*;

public class UpdateableTreeMap {

    private final HashMap<Player, PlayerData> hashMap;
    private final TreeMap<Player, PlayerData> treeMap;

    public UpdateableTreeMap() {
        hashMap = new HashMap<>();
        treeMap = new TreeMap<>(new ValueComparator(hashMap));
    }

    public Map<Player, PlayerData> internalMap() {
        return hashMap;
    }

    public Set<Player> keySet() {
        return hashMap.keySet();
    }

    public boolean containsKey(Object key) {
        return hashMap.containsKey(key);
    }

    public PlayerData get(Object key) {
        return hashMap.get(key);
    }

    public PlayerData remove(Object key) {
        treeMap.remove(key);
        return hashMap.remove(key);
    }

    public boolean isEmpty() {
        return hashMap.isEmpty();
    }

    public int size() {
        return hashMap.size();
    }

    public Map.Entry<Player, PlayerData> firstEntry() {
        return treeMap.firstEntry();
    }

    public Set<Map.Entry<Player, PlayerData>> entrySet() {
        return hashMap.entrySet();
    }

    public Set<Map.Entry<Player, PlayerData>> sortedEntrySet() {
        return treeMap.entrySet();
    }

    public Collection<PlayerData> values() {
        return hashMap.values();
    }

    public Collection<PlayerData> sortedValues() {
        return treeMap.values();
    }

    public PlayerData put(Player key, PlayerData value) {
        hashMap.put(key, value);
        return treeMap.put(key, value);
    }

    public void update(Player key) {
        PlayerData value = treeMap.remove(key);

        if (value != null) {
            treeMap.put(key, value);
        }
    }

    public static class ValueComparator implements Comparator<Player> {

        private final Map<Player, PlayerData> map;

        public ValueComparator(Map<Player, PlayerData> map) {
            this.map = map;
        }

        public int compare(Player o1, Player o2) {
            if (o1 == o2)
                return 0;

            PlayerData d1 = map.get(o1);
            PlayerData d2 = map.get(o2);

            System.out.println(o1.getName() + " " + d1.maxhealth + " - " + d2.maxhealth + " " + o2.getName());
            System.out.println("Result: " + (o1 == o2 ? 0 : (d1.maxhealth < d2.maxhealth ? 1 : -1)));

            if (d1.maxhealth < d2.maxhealth)
                return 1;
            return -1;
        }

    }

}

When I call update(Player), I can clearly see thanks to the System.out.println() lines that compare(Player, Player) is returning -1. Yet, when I loop through the TreeMap using the sortedValues() method, the order is incorrect.

Chiseled
  • 2,280
  • 8
  • 33
  • 59
Elite_Dragon1337
  • 348
  • 2
  • 13
  • What order do you want ? Your code gives "most health" first. If it is not what you want, change the sign in your comparator. – Mathias Aboudou Aug 13 '15 at 18:26
  • Yes, that's the ordering I'm looking for. However, it is not consistent at all. – Elite_Dragon1337 Aug 13 '15 at 18:27
  • Trying to sort a map by values is, in most ways, doomed from the start. The Comparator hack you're doing is just going to paper over the mess. http://stackoverflow.com/a/2581754/869736 is really the only reliable way to solve this problem. – Louis Wasserman Aug 13 '15 at 18:32

2 Answers2

2

Per Treemap API, TreeMap.values() returns values IN THE ORDER OF KEYS, not values.

public Collection values()

Returns a Collection view of the values contained in this map.

The collection's iterator returns the values in ascending order of the corresponding keys. The collection's spliterator is late-binding, fail-fast, and additionally reports Spliterator. ORDERED with an encounter order that is ascending order of the corresponding keys.

The collection is backed by the map, so changes to the map are reflected in the collection, and vice-versa. If the map is modified while an iteration over the collection is in progress (except through the iterator's own remove operation), the results of the iteration are undefined. The collection supports element removal, which removes the corresponding mapping from the map, via the Iterator.remove, Collection.remove, removeAll, retainAll and clear operations. It does not support the add or addAll operations.

Specified by: values in interface Map Specified by: values in interface SortedMap Overrides: values in class AbstractMap Returns: a collection view of the values contained in this map

Hint: You can sort TreeMap.values() at additional cost.

Y.H.
  • 33
  • 1
  • 8
  • My comparator sorts the Map by its value though, by retrieving the value first using the key. – Elite_Dragon1337 Aug 13 '15 at 18:34
  • 1
    Your comparator is broken in a couple of ways, including: if two `PlayerData`s have the same `maxhealth`, then they both compare as greater than each other; also, the comparison can change over time, which `TreeMap` cannot reflect. – Louis Wasserman Aug 13 '15 at 18:50
1

I gave up and ended up switching to Google's TreeMultiMap

Elite_Dragon1337
  • 348
  • 2
  • 13