[11] RFR: 8193128: Reduce number of implementation classes returned by List/Set/Map.of()

Claes Redestad claes.redestad at oracle.com
Mon Jan 8 20:57:00 UTC 2018


Gosh! My intent was to carry over AbstractSet::equals verbatim, so I 
have no idea how the size
check got lost in translation and passed through testing! Appears to be 
another hole in the test
coverage, or I didn't run the right ones.

As to calculating o.size() as we go then I'm not sure: most Set::size() 
impls in the JDK are O(1)
(including the ones here), and you lose a very fast fast path in common 
cases for a hypothetical
improvement on some collection types whose size() isn't. Leaving it as 
is is also sticking with
the status quo, which seems favorable for the scope of this RFE.

CCE is specified to be thrown by Set::contains/containsAll, so I think 
we should play it safe and
leave it unchanged.

Thanks!

/Claes

On 2018-01-08 20:38, Peter Levart wrote:
> Also, I don't think that ClassCastException should be caught here. It 
> should never be thrown by containsAll(c) anyway.
>
> On 01/08/18 20:09, Peter Levart wrote:
>> Hi Claes,
>>
>> Are you sure that AbstractImmutableSet.equals(Object) is correct?
>>
>>         @Override
>>         public boolean equals(Object o) {
>>             if (o == this)
>>                 return true;
>>
>>             if (!(o instanceof Set))
>>                 return false;
>>             Collection<?> c = (Collection<?>) o;
>>             try {
>>                 return containsAll(c);
>>             } catch (ClassCastException | NullPointerException unused) {
>>                 return false;
>>             }
>>         }
>>
>>
>> It seems to me that this implementation returns true when passing in 
>> a sub-set of this. Should the method also check that the size(s) of 
>> both sets are the same?
>>
>> Regards, Peter



More information about the core-libs-dev mailing list