On Wed, 15 Sep 2021 16:13:56 GMT, Daniel Fuchs <dfu...@openjdk.org> wrote:
>> src/java.base/share/classes/java/util/Properties.java line 932:
>>
>>> 930: if (entries instanceof Collections.SynchronizedSet<?> ss
>>> 931: && ss.c instanceof EntrySet) {
>>> 932: entries = new ArrayList<>(entries);
>>
>> From the description referring to a 'different instance', I expected to see
>> == or != in this test.
>> Since Properties.entrySet() always returns a new synchronized set of a new
>> EntrySet,
>> specifying that the underlying map is the same instance is more difficult.
>> Perhaps either the SynchronizedSet or the EntrySet instance could be cached
>> and compared.
>>
>> (typo: missing space after period... ".If yes")
>
> A custom subclass couldn't create an instance of EntrySet directly. Therefore
> if it wants to reorder the entries, it would have to return a new TreeSet or
> LinkedHashSet - or some other custom set implementation. If the result of
> calling entrySet() is an EntrySet wrapped in an UnmodifiableSet it's
> therefore legitimate to assume that the set hasn't been reordered, and that
> we can reorder it. Or am I missing something?
I've updated the PR to clarify what this part of the code is doing. I took
Daniel's input to reword the javadoc as well as tried to improve the code
comment where this check is done. Essentially, we check for the "type" of the
returned intstance and if we see that it's a `SynchronizedSet` whose underlying
collection is a private class `EntrySet` then we know that it's the one created
by Properties.entrySet(). Like Daniel says, subclasses won't be able to create
or use this internal private class and if they did override the entrySet() to
change the order of the entries they would have to do it by returning us a
different "type" from that method.
-------------
PR: https://git.openjdk.java.net/jdk/pull/5372
