ImmutableSortedMap - Duplicate keys in mappings

Question

I have an ImmutableSortedMap that holds data in the following datastructure.

<String, Pair<Long, Double>>

My goal is to sort this map in descending order starting with the largest Double and if two Doubles are the same, sort it by the Long. If both the Double and Long are the same, sort it by String

My current comparator only deals with the first case, and looks like this.

private ImmutableSortedMap<String, Pair<Long, Double>> sortInDescendingSizeOrder(final Map<String, Pair<Long, Double>> statsByType) {
    Ordering<String> ordering = Ordering
            .from(new Comparator<Pair<Long, Double>>() {
                @Override
                public int compare(Pair<Long, Double> o, Pair<Long, Double> o2) {
                    return o.getRight().equals(o2.getRight()) ? o.getLeft().compareTo(o2.getLeft()) : o.getRight().compareTo(o2.getRight());
                }
            })
            .reverse()
            .onResultOf(Functions.forMap(statsByType, null));

    return ImmutableSortedMap.copyOf(statsByType, ordering);
}

However, it finds two Doubles that are the same and throws the following exception: Exception in thread "main" java.lang.IllegalArgumentException: Duplicate keys in mappings

I don't understand what am I doing wrong...

EDIT: I have tried to add the compound method and order by String (at least).

.compound(new Comparator<String>() {
                    @Override
                    public int compare(String o1, String o2) {
                        return o1.compareTo(o2);
                    }
                });

This makes the code not throw any Exception, however, there is no ordering whatsoever in the resulted map.

EDIT 2: At this point, any ordering like the one described above of a

Map<String, Pair<Long, Double>>

will do. Doesn't necessarily need to be an ImmutableSortedMap

score 3 · Accepted Answer · edited May 23 '17 at 11:52

EDIT: See Louis' answer for actual proper solution (ImmutableMap.Builder#orderEntriesByValue(Comparator)). Continue reading this answer for fix of an incorrect approach ;)

It's a documented behavior of ImmutableSortedMap#copyOf(Map, Comparator):

Throws:

NullPointerException - if any key or value in map is null
IllegalArgumentException - if any two keys are equal according to the comparator

What you get is a result of values being equal according to comparator, so the problem is there. See this answer for comparing map by value - as Louis mentions (he's from Guava team), using Functions.forMap is tricky.

In your case adding .nullsLast() to first ordering and appending .compound(Ordering.natural()) should work. For example:

Map<String, Pair<Long, Double>> map = ImmutableMap.of(
        "a", Pair.of(1L, 1.0d),
        "b", Pair.of(1L, 1.0d),
        "c", Pair.of(1L, 1.0d),
        "d", Pair.of(1L, 1.0d)
);

@Test
public void should2()
{
    final ImmutableSortedMap<String, Pair<Long, Double>> sortedMap = sortInDescendingSizeOrder(map);
    assertThat(sortedMap).hasSize(4);
    System.out.println(sortedMap); // {a=(1,1.0), b=(1,1.0), c=(1,1.0), d=(1,1.0)}
}

and for

Map<String, Pair<Long, Double>> map = ImmutableMap.of(
        "a", Pair.of(1L, 1.0d),
        "b", Pair.of(2L, 1.0d),
        "c", Pair.of(1L, 2.0d),
        "d", Pair.of(2L, 2.0d)
);

it outputs {d=(2,2.0), b=(2,1.0), c=(1,2.0), a=(1,1.0)}.

(sortInDescendingSizeOrder for reference)

private ImmutableSortedMap<String, Pair<Long, Double>> sortInDescendingSizeOrder(
        final Map<String, Pair<Long, Double>> statsByType) {
    Ordering<String> ordering = Ordering.from(new Comparator<Pair<Long, Double>>() {
                @Override
                public int compare(Pair<Long, Double> o, Pair<Long, Double> o2) {
                    return ComparisonChain.start()
                            .compare(o.getRight(), o2.getRight())
                            .compare(o.getLeft(), o2.getLeft())
                            .result();
                }
            })
            .reverse()
            .nullsLast()
            .onResultOf(Functions.forMap(statsByType, null))
            .compound(Ordering.natural());

    return ImmutableSortedMap.copyOf(statsByType, ordering);
}

ComparisonChain is another goodie from Guava you can use here.

Ordering is still messed up... There is absolutely no ordering, not on the Double, Long or String — Adrian Daniel Culea, Nov 22 '16 at 11:23
I edited answer and added input map containing `{a=(1,1.0),b=(2,1.0),c=(1,2.0),d=(2,2.0)}` and output looks good to me: `{d=(2,2.0), b=(2,1.0), c=(1,2.0), a=(1,1.0)}`. Can you post sample data you put in and what's your expected output? — Xaerxess, Nov 22 '16 at 11:27
The resulting map is still going to throw on `get(keyNotInMap)`. Using `ImmutableSortedMap` at all is not really a good idea here. — Louis Wasserman, Nov 22 '16 at 16:05
@LouisWasserman Yep, `.nullsLast()` was missing. As as cited, it's tricky - added reference to your simpler answer. — Xaerxess, Nov 22 '16 at 16:54

Louis Wasserman · Answer 2 · 2016-11-22T17:59:19.600

ImmutableSortedMap is not the correct API for this. ImmutableSortedMap is supposed to be sorted by keys, and that's clearly not what you're doing.

What would make more sense is writing

new ImmutableMap.Builder<String, Pair<Long, Double>>()
   .orderEntriesByValue(comparing(Pair::getSecond).thenComparing(Pair::getFirst))
   .putAll(map)
   .build();

though, if you can't use Java 8, you'll have to explicitly write the comparator

new Comparator<Pair<Long, Double>>() {
  @Override public int compare(Pair<Long, Double> a, Pair<Long, Double> b) {
    return ComparisonChain.start()
        .compare(a.getSecond(), b.getSecond())
        .compare(a.getFirst(), b.getFirst())
        .result();
  }
}

Regarding `ImmutableMap.Builder#orderEntriesByValue` - that neat (haven't seen this, it's `@since 19.0`)! OP wants sort on both pair values and seems to be on Java <7, so `ComparisonChain` seems right here. — Xaerxess, Nov 22 '16 at 16:56

ImmutableSortedMap - Duplicate keys in mappings

2 Answers2