Looks good to me. I will run it through final integration tests and push to TL.
Mike On May 1 2013, at 15:55 , Akhil Arora wrote: > On 04/26/2013 04:52 AM, Paul Sandoz wrote: >> >> On Apr 24, 2013, at 7:57 PM, Remi Forax <fo...@univ-mlv.fr> wrote: >> >>> On 04/24/2013 07:24 PM, Akhil Arora wrote: >>>> On 04/24/2013 06:19 AM, Alan Bateman wrote: >>>>> On 23/04/2013 20:18, Akhil Arora wrote: >>>>>> On 04/22/2013 11:42 AM, Alan Bateman wrote: >>>>>>> One thing I meant to ask when forEachRemaining was added was whether it >>>>>>> should say anything about the "last element"? I see in the webrev that >>>>>>> you've set it for the ArrayList iterators but not the LinkedList >>>>>>> iterator - should it? >>>>>> >>>>>> done, and added more tests for the state of the iterator after >>>>>> forEachRemaining returns >>>>>> >>>>>> http://cr.openjdk.java.net/~akhil/8005051.2/webrev/ >>>>>> >>>>>> (a portion of version 1 of the webrev has already been pushed to TL as >>>>>> part of rev 6973 http://hg.openjdk.java.net/jdk8/tl/jdk/rev/98a7bb7baa76) >>>>> The remaining changes in the webrev looks okay to me. Also good to have >>>>> the test expanded. >>>>> >>>>> So do you think that the javadoc should say anything about the >>>>> relationship between forEachRemaining and remove? >>>> >>>> Good question. forEachRemaining docs state - >>>> >>>> Implementation Requirements: >>>> >>>> The default implementation behaves as if: >>>> >>>> while (hasNext()) >>>> action.accept(next()); >>>> >>>> so a subsequent remove() should remove the last element. >>>> >>>> To avoid blocking the feature, I have filed >>>> https://jbs.oracle.com/bugs/browse/JDK-8013150 to refine the behavior of >>>> remove and other ListIterator methods after forEachRemaining returns. >>> >>> I think the fact that the last element can be removed should be specified >>> as unspecified, >>> because it's more an artefact of the default implementation than something >>> which part >>> of the semantics. >>> >> >> I was wondering about that too. What happens if an implementing class >> decides later on to override the default implementation? If the overriding >> implementation does not conform to the same behaviour as the default it >> could break compatibility. >> >> Plus one could change code from: >> >> while(it.hasNext() { doSomething(it.next()); } >> doSomethingAtEnd(it); >> >> to >> >> it.forEachRemaining(this::doSomething} >> doSomethingAtEnd(it); >> >> Paul. > > The testOptimizedForEach test verifies that remove() and previous() work as > expected after forEachRemaining returns (lines 238-251). Please review - > > http://cr.openjdk.java.net/~akhil/8013150/webrev/ > > Somehow tests got left behind in the previous commit, adding them back. > > Thanks
