RFC: draft API for JEP 269 Convenience Collection Factories (original) (raw)

Stuart Marks stuart.marks at oracle.com
Fri Oct 9 23:11:09 UTC 2015


On 10/9/15 6:11 AM, Stephen Colebourne wrote:

On 9 October 2015 at 00:39, Stuart Marks <stuart.marks at oracle.com> wrote:

1. Number of fixed arg overloads. Guava follows this pattern: of(T) of(T, T) of(T, T, T) of(T, T, T, T... elements) whereas the proposal has of(T) of(T, T) of(T, T, T) of(T... elements) I'd be interested to know why Guava did it that way and what the trade offs are.

I can't answer for the Guava guys, of course, but I can supply some additional background about why we chose this approach for our proposal.

Note carefully that the Guava pattern is:

 of()                    // zero args, for completeness
 of(T)
 of(T, T)
 of(T, T, T)
 of(T, T, T, T, T...)    // an extra fixed arg THEN the varargs param

Following this pattern means that overload resolution can be performed entirely based on the arity of the call site.

With the JEP 269 proposal, there is a potential overloading ambiguity with these two methods:

 of(T)
 of(T...)

The reason we chose to provide a one-arg of(T...) method is that it supports a secondary use case, which is how to create one of these collections if the values aren't known until runtime. The of(T...) method allows you to pass an arbitrary number of actual arguments as well as an array of arbitrary size. It's really hard to do this with the Guava pattern of fixed and varargs methods.

Now, Guava handles this use case by providing a family of copying factories that can accept an array, a Collection, an Iterator, or an Iterable. These are all useful, but for JEP 269, we wanted to focus on the "collection literal like" APIs and not expand the proposal to include a bunch of additional factory methods. Since we need to have a varargs method anyway, it seemed reasonable to arrange it so that it could easily accept an array as well.

Now, the usual problem with this scheme is that calling of(null) is ambiguous. Indeed, it actually is ambiguous in the JEP 269 proposal, and call sites will generate varargs warnings. It turns out that we've finessed this issue since we disallow nulls.

Another issue is that a call site that passes a T[] argument matches the of(T...) overload and not of(T), where in the latter case T is an array type. This is a problem if you want to create a list or set containing a single array. If you need this, you can do

 List<String[]> list = List.<String[]>of(stringArray);

This is messy, but it really seems like an edge case, and there's a simple workaround. I've described this in an @apiNote on List.of(T...). (I need to add something similar for Set.of(T...).)

2. Other concrete collection factories.

I've chosen to provide factories for the concrete collections ArrayList, HashSet, and HashMap, since those seem to be the most commonly used. Is there a need to provide factories for other concrete collections, such as LinkedHashMap? LinkedHashMap definitely LinkedList definitely not (as its very slow and use should not be encouraged). TreeSet/TreeMap, maybe, they'd need an extra parameter though.

LinkedHashMap: ok. I assume this should create an insertion-ordered map, with the initial entries being inserted in left-to-right order.

Does anybody care about LinkedHashSet?

3. Duplicate handling.

My current thinking is for the Set and Map factories to throw IllegalArgumentException if a duplicate element or key is detected. Definitely.

Well ok then! :-)

Given that ofEntries() takes a Map.Entry as input, why does Map.KeyValueHolder need to be public? This would require Map.entry(K,V) return Map.Entry, not Map.KeyValueHolder.

Good question, it might or might not need to be this way. By having Map.entry() return KeyValueHolder instead of Map.Entry, call sites are compiled with KeyValueHolder as the method return type. If, in the future, KeyValueHolder were to become a value type, and in the future, if another overload

 Map.ofEntries(KeyValueHolder... entries)

were added, then recompiling will avoid upcasting to Map.Entry, which might prevent boxing of the value type.

I think the way this needs to be set up and how it will interact with some future evolution toward value types is mainly in Brian Goetz' head right now, so I'll need to go over this again to make sure it's set up correctly.

s'marks

My thinking was, suppose KeyValueHolder eventually turns into a value type



More information about the core-libs-dev mailing list