Hi Tagir!
I think what you have in the last webrev looks good!
In CopiesList.equals() please use eq() instead of Objects.equal() (see a
comment at the line 5345).
With kind regards,
Ivan
On 12/3/18 9:22 PM, Tagir Valeev wrote:
Hello!
Thank you for your comments!
Yes, deserialization will be broken if we assume that size is never 0.
Also we'll introduce referential identity Collections.nCopies(0, x) ==
Collections.nCopies(0, y) which might introduce slight semantics
change in existing programs. Once I suggested to wire Arrays.asList()
(with no args) to Collections.emptyList(), but it was rejected for the
same reason: no need to introduce a risk of possible semantics change.
I updated webrev with equals implementation and test:
http://cr.openjdk.java.net/~tvaleev/webrev/8214687/r2/
Comparing two CopiesList is much faster now indeed. Also we can spare
an iterator in the common case and hoist the null-check out of the
loop. Not sure whether we can rely that JIT will always do this for
us, but if you think that it's unnecessary, I can merge the loops
back. Note that now copiesList.equals(arrayList) could be faster than
arrayList.equals(copiesList). I don't think it's an issue. On the
other hand we could keep simpler and delegate to super-implementation
if the other object is not a CopiesList (like it's implemented in
java.util.RegularEnumSet::equals for example). What do you think?
With best regards,
Tagir Valeev.
On Tue, Dec 4, 2018 at 10:56 AM Stuart Marks <[email protected]> wrote:
I believe it makes sense to override CopiesList.equals()
Also: contains(), iterator(), listIterator()
equals(): sure
contains() is already overridden. Not sure there's much benefit to overriding
the iterators.
s'marks
--
With kind regards,
Ivan Gerasimov
