Martin,

On 2018-05-14 16:15, Martin Buchholz wrote:
Claes,

Looks good.

thanks!


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         }

Sure.


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         }


Perhaps you misread as I think the patch does what I think you're suggesting and only checks once. True, this is subtly
different in behavior from the current implementation
which is fail-fast.

While it shouldn't matter semantically, it could in theory mean a measurable delay in throwing a CME when dealing with large lists.  As CMEs in ArrayLists could be considered a coding error - and the speedup of only checking once for a bulk operation like equal is significant - I think it's a reasonable trade-off to go ahead with this.

/Claes

Reply via email to