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

Peter Levart peter.levart at gmail.com
Tue Jan 9 12:27:19 UTC 2018


Hi Claes,

On 01/09/2018 02:17 AM, Claes Redestad wrote:
> If we took it a step further we could save us the trouble of throwing 
> and swallowing NPEs in
> AbstractImmutableSet::equals when the other set contains null, which 
> might be an optimization
> in that case:
>
>         @Override
>         public boolean equals(Object o) {
>             if (o == this)
>                 return true;
>
>             if (!(o instanceof Set))
>                 return false;
>
>             Collection<?> c = (Collection<?>) o;
>             if (c.size() != size())
>                 return false;
>
>             for (Object e : c)
>                 if (e == null || !contains(e))
>                     return false;
>             return true;
>         }
>
> It might be rare enough that it's not worth bothering with, though. 

That's nicer in my opinion. What to do with ClassCastException from 
contains(e)? Perhaps just document that any future implementation that 
might throw it from contains() should also modify the equals() method 
and catch it there.

And if you already do explicit iteration over elements, you could as 
well count them and do the size check with counted size at the end. 
Imagine the following scenario:

ConcurrentHashMap m = new ChocurrentHashMap();
m.put(1, 1);
m.put(2, 2);
m.put(3, 3);

Set s = Set.of("a", "b", "c");

Thread 1:

s.equals(m.keySet())

Thread 2:

m.clear();

It is surprising when Thread 1 encounters 'true' as the result of the 
equals() above which can happen when code checks for sizes separately. 
Iterating CHM.keySet() does not provide a snapshot of the content 
either, but counting iterated and compared elements nevertheless 
exhibits less surprising behavior. I don't know if it matters though.

Regards, Peter



More information about the core-libs-dev mailing list