RFR [15] 8161558: ListIterator should not discard cause on exception

[Stuart Marks]

On 1/15/20 4:46 AM, Martin Buchholz wrote:
> I would have documented whitebox test assumptions: that nCopies iterators
> are implemented via AbstractList, and that AbstractList's list iterator
> inherits behavior from iterator.
> 
> I probably would have added a plain iterator test, and might have
> refactored the code that tests the exception.

Hm. Thinking about this a bit more, it seems to me that this is a concern that 
we ought to deal with more effectively than just documenting it. The risk is 
that Collections.nCopies is reimplemented differently, which would render the 
useless as it's not testing the thing that it's intended to test. It would 
probably still pass, so nobody would ever look at it and see a comment 
documenting the assumptions, or even if they did they might not grasp the 
implications.

Fortunately it's possible to implement a concrete List implementation using 
AbstractList with just a few lines of code, so I think we should go ahead and do 
that.

For a similar reason I also agree that it would be wise to add a test of the 
plain Iterator as well as the ListIterator. Even though we "know" it's 
redundant, if the AbstractList iterator code were refactored in the future, it 
might invalidate assumptions the test is making.

I usually draw the refactoring line between two and three. With two cases I 
think a little duplication is sometimes reasonable, as the overhead of 
introducing sharing (both in the conceptual overhead of adding an abstraction 
layer, and the syntactic overhead of adding another method) offsets the savings. 
With three cases the case for extracting common is much stronger. Making the 
assertions be in common code will lose some customization of messages, but I 
don't think that's a great loss; there's already enough information to pinpoint 
which case is failing.

s'marks