RFR: 8196340: (coll) Examine overriding inherited methods in ArrayList and ArrayList.SubList (original) (raw)

Martin Buchholz martinrb at google.com
Mon May 14 14:15:11 UTC 2018


Claes,

Looks good.

I would move the size check up to the beginning of the method.

583 int expectedModCount = modCount; 584 int otherModCount = other.modCount; 585 int s = size; 586 if (s != other.size) { 587 return false; 588 }

I would prefer having only one comodification check for a bulk operation, but I understand that checking at each step is more compatible with the default implementation.

594 for (int i = 0; i < s; i++) { 595 if (!Objects.equals(es[i], otherEs[i])) { 596 other.checkForComodification(otherModCount); 597 checkForComodification(expectedModCount); 598 return false; 599 } 600 }

On Mon, May 14, 2018 at 6:37 AM, Claes Redestad <claes.redestad at oracle.com> wrote:

Hi Ivan,

right, checkForComodification() alone should be sufficient here. Updated in-place: http://cr.openjdk.java.net/~redestad/8196340/open.01/ Thanks! /Claes

On 2018-05-12 03:38, Ivan Gerasimov wrote: Hi Claes!

One thing I can't figure out is why both these two checks are necessary: 1303 checkForComodification(); 1304 root.checkForComodification(expectedModCount); The former compares the current root.modCount with the one at the time this subList was created. The later one compares the current root.modCount with the one at the time the method was called. If the later fails, wouldn't it imply the former should also have failed?

With kind regards, Ivan



More information about the core-libs-dev mailing list