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
