RFC: draft API for JEP 269 Convenience Collection Factories

[Stuart Marks]

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