+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
<jbluettdun...@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 <patr...@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 <patr...@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 <patr...@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
