> On 4 Mar 2016, at 15:38, Tagir F. Valeev <amae...@gmail.com> wrote: > > Hello! > > Thank you for the review! >
Thanks. I just realised there are some subtleties where if the top-level list is reduced in size the spliterator of a sublist may on traversal throw an ArrayIndexOutOfBoundsException rather than ConcurrentModificationException. This can already occur for a partially traversed top-level list spliterator, so i am wondering how much we should care. Arguably in the sublist case there is another level of indirection, where errors creep in before traversal, so it suggests we should additionally check the mod count at the start of the traversal methods, and it’s probably ok on a best-effort basis to do this for the sublist spliterator and not it’s splits. Separately, i am not sure why the SubList.iterator has to check that the offset + 1 is within bounds, since expectedModCount = ArrayList.this.modCount should be false. Paul. > Here's updated webrev: > http://cr.openjdk.java.net/~tvaleev/webrev/8148748/r3/ > > PS> Looks good. I especially like: > > PS> 125 addCollection(l.andThen(list -> list.subList(0, > list.size()))); > > PS> Can you also update SpliteratorTraversingAndSplittingTest? > > PS> void addList(Function<Collection<T>, ? extends List<T>> l) { > PS> // @@@ If collection is instance of List then add sub-list tests > PS> addCollection(l); > PS> } > > Done. > >>> Now it's separate anonymous class as you suggested. >>> ArrayListSpliterator is untouched. Note that trySplit() can return >>> original ArrayListSpliterator as after the binding their behavior is >>> compatible. >>> > > PS> Very nice, might be worth an extra comment noting that. Up to you. > > Short comments added. > > With best regards, > Tagir Valeev. >
