Skip to content

ImmutableMap.sortedCopyOf #8552

Description

@cpovirk

This comes as a request from an external user. The motivation is to produce a deterministic order while continuing to use hashing for lookups, rather than the O(log N) lookups of ImmutableSortedMap. It is of course already possible to sort before calling ImmutableMap.copyOf, but a method on ImmutableMap might catch users' attention more, prompting them to consider whether sorting is right for them. (As usual, prompting users to consider something can be a small negative in the large majority of cases but a sizable positive in a small number of cases, and that can sometimes be OK. Or maybe users who are concerned enough should soft-ban ImmutableSortedMap and perhaps ImmutableSortedSet, too?) It might also perform better than separate sort+copy operations would, especially if the input is a collection whose stream implementation we have no hope of optimizing. I do see plenty of users of sorted().collect(toImmutableMap(...)), so those are people who either (depending on your perspective) want this or don't need this. [edit: And also as usual, we could worry that users will stress over whether to use sorted().collect(toImmutableMap(...)) or sortedCopyOf in cases where it doesn't matter.]

The new API would be similar to the existing ImmutableList.sortedCopyOf(Iterable), and it could come with the same kind of Comparator-accepting overload. (It's admittedly a little strange that a method named "ImmutableMap.sortedCopyOf" would not return the ImmutableSortedMap class, whereas there is no ImmutableSortedList.)

It also reminds me a bit of how ImmutableMultimap.Builder offers orderKeysBy and orderValuesBy, which I think we envisioned half as a partial replacement for having an ImmutableSortedKeyMultimap, etc. and half as a way of performing a sorting step that today we'd perform with Stream.

This proposal raises a question as to whether we'd want, well, possibly the sort of Builder API that I'd mentioned, but more likely whether we'd want similar sortedCopyOf methods on ImmutableSet, etc.

There is probably a discussion to be had around the idea that ImmutableMap uses hashing for deduplication. For users of buildKeepingLast(), this raises questions about whether the sorting happens first or the deduplication happens first. This is in contrast to the Stream alternative of input.stream().sorted(...).collect(toImmutableMapKeepingLast()) [edit: Oops, that exists only in an internal "contrib" package.], which makes the ordering clear. (Not that I'd expect this to be a major issue, just something for us to consider, test, and document.)

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions