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
