Adrian Daniel Culea
Adrian Daniel Culea

Reputation: 181

ImmutableSortedMap - Duplicate keys in mappings

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

Upvotes: 0

Views: 846

Answers (2)

Grzegorz Rożniecki
Grzegorz Rożniecki

Reputation: 28015

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.

Upvotes: 3

Louis Wasserman
Louis Wasserman

Reputation: 198211

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();
  }
}

Upvotes: 3

Related Questions