RFR: JDK-8134373: explore potential uses of convenience factories within the JDK

[Claes Redestad]

+1

I don't mind List.of() aesthetically, but there are places where
startup/footprint is important where Collections.emptyList()
is simply superior, e.g., constituting permanent data structures
such as the module graph during early bootstrap.

I know there are some drawbacks to the singleton approach of
Collections.emptyList() (forces a potentially expensive memory
load instead of just quickly allocating something that most likely
will be thrown away just as quick), but having a very limited set
of known constants for such things should be hard to notice on
any workload since they'll be very likely to be in a CPU cache.

Also saves having a few extra classes and decreases chances for
profile pollution when calling methods with both
Collections.emptyList() and List.of().

TL;DR: I think List.of() should be an alias for Collections.emptyList()
(same for Set.of() <-> Collections.emptySet(), Map.of() ..).

Sorry. :-)

/Claes

On 2016-09-15 13:06, Daniel Fuchs wrote:
> Hi,
>
> I'm not sure I like the replacement of Collections.emptyList()
> by List.of();
> I find emptyList() more expressive (+ it always returns
> the same instance).
>
> best regards,
>
> -- daniel
>
> On 15/09/16 11:46, Pavel Rappo wrote:
>> Hi,
>>
>> I have had a look at the change. It looks good.
>>
>> Retrofitting Collections.unmodifiableXXX/Arrays.asList with 
>> Convenience Factory
>> Methods requires extra carefulness as the factory methods are 
>> stricter than any
>> of those. Not only do they provide unconditional non-modifiability 
>> [0], they
>> also do not tolerate `null` elements.
>>
>> It looks like you took all that into account.
>> Tiny little comments and suggestions.
>>
>> 1. It might be the case we no longer need this [1]:
>>
>>     finderList.forEach(Objects::requireNonNull);
>>
>> as the List.of does the same thing for free. Though it might be okay 
>> to leave it
>> as an explicit check.
>>
>> Also, could you please fix a typo here (the same file):
>>
>>     * will be propogated to the caller of the resulting module finder's
>>                   ^
>> 2. An interesting question (though it's a completely different issue) 
>> is whether
>> or not the `cookieHeader` list in the map should also be unmodifiable 
>> [2]?
>>
>> 3. Could you please also fix the same (copy-pasted?) typo in 
>> FileTreeWalker?
>>
>>     if {@code options} is {@ocde null} or the options
>>                             ^^
>> 4. Please remove this line from the ZoneRules constructor's javadoc:
>>
>>     @return the zone rules, not null
>>
>> 5. Maybe we should revisit javadoc on those fields in [3]:
>>
>>     This <code>List</code> is {@linkplain 
>> Collections#unmodifiableList(List) unmodifiable}.
>>
>> Since it's no longer the case.
>>
>> 6. I couldn't find any evidence that `resolverFields` might contain 
>> `null`.
>> Am I missing a javadoc or a line of code? Maybe we should talk to 
>> java.time
>> folks to see if it's the case?
>>
>> 7. Try to not make lines longer than they were before: 80 characters. 
>> Unless
>> it's really needed.
>>
>> Thanks,
>> -Pavel
>>
>> -------------------------------------------------------------------------------- 
>>
>> [0] Compare List.of().remove(new Object()) with 
>> Collections.emptyList().remove(new Object())
>> [1] 
>> http://cr.openjdk.java.net/~reinhapa/reviews/8134373/webrev.00/src/java.base/share/classes/java/lang/module/ModuleFinder.java.sdiff.html
>> [2] 
>> http://cr.openjdk.java.net/~reinhapa/reviews/8134373/webrev.00/src/java.base/share/classes/java/net/CookieManager.java.sdiff.html
>> [3] 
>> http://cr.openjdk.java.net/~reinhapa/reviews/8134373/webrev.00/src/java.base/share/classes/java/util/ResourceBundle.java.sdiff.html
>>
>>> On 15 Sep 2016, at 09:52, Jonathan Bluett-Duncan 
>>> <jbluettduncan at gmail.com> wrote:
>>>
>>> Thanks Patrick!
>>>
>>> Stuart, would you be happy to sponsor this change?
>>>
>>> If anyone else has any thoughts regarding this change, then I'm all 
>>> ears.
>>>
>>> Bug link: https://bugs.openjdk.java.net/browse/JDK-8134373
>>> Rationale for changes:
>>> http://mail.openjdk.java.net/pipermail/core-libs-dev/2016-September/043484.html 
>>>
>>>
>>> Kind regards,
>>> Jonathan
>>>
>>>
>>> On 14 September 2016 at 21:55, Patrick Reinhart <patrick at reini.net> 
>>> wrote:
>>>
>>>> Hi Jonathan,
>>>>
>>>> Here are your changes in a webrev:
>>>>
>>>> http://cr.openjdk.java.net/~reinhapa/reviews/8134373/webrev.00
>>>>
>>>> -Patrick
>>>>
>>>> Am 13.09.2016 um 14:45 schrieb Patrick Reinhart <patrick at reini.net>:
>>>>
>>>> Hi  Jonathan,
>>>>
>>>> On 2016-09-13 02:13, Jonathan Bluett-Duncan wrote:
>>>>
>>>> Hi Patrick,
>>>> Thank you very much for the offer to hold my patch for me!
>>>>
>>>>
>>>> No problem.
>>>>
>>>> Is it common practice to send patches to others using PGP?
>>>>
>>>>
>>>> No, this is my personal setting.
>>>>
>>>> Kind regards,
>>>> Jonathan
>>>> On 12 September 2016 at 21:08, Patrick Reinhart <patrick at reini.net> 
>>>> wrote:
>>>>
>>>> Hi Jonathan,
>>>> As I just also wanted to help some more clean-up in the JDKs final 
>>>> phase, I
>>>> could offer you to hold that patch. Just send it to me and I will 
>>>> prepare
>>>> the webrev for you….
>>>> -Patrick
>>>>
>>>>
>>>> -Patrick
>>>>
>>>>
>>>>
>>
>