RFR(s): 8203670: unmodifiable List iterator() implementations should not be ListIterators

[Peter Levart]

On 06/26/2018 08:11 AM, Zheka Kozlov wrote:
> I agree with Ivan. Isn't it better to create a standalone implementation of
> Iterator and return its instance in iterator()? Implementing of Iterator
> for ImmutableList doesn't look like a big problem.

Also, performance would be better if it was a separate class. It could 
be a superclass of ListItr so that common logic is not duplicated.

@Stable annotation on private final boolean isListIterator does not help 
here unless the returned Iterator object is assigned to a static final 
field and used from it. Hardly a common use case for Iterator(s) I guess.

Another thing about @Stable is that it never has effect on fields that 
hold a default value. Ok, when isListIterator == false, we don't need 
speed as we're throwing UOE anyway.

The same arguments hold for other two @Stable fields in this class. I 
don't think those three @Stable annotations have any effect on real 
Iterator use cases.

Regards, Peter

>
> 2018-06-26 6:14 GMT+07:00 Ivan Gerasimov <ivan.gerasimov at oracle.com>:
>
>> Hi Stuart!
>>
>> Out of curiosity.  What if someone does something like
>>
>> if (it instanceof ListIterator) {
>>      // optimized for bidirectional access
>> } else {
>>      // slower algorithm
>> }
>>
>> previous(), nextIndex() and previousIndex() methods are not declared to be
>> optional, so is it appropriate to throw UnsupportedOperationException from
>> them?
>>
>> Someone may assume that if an object can be cast to ListIterator
>> interface, non-optional methods should not throw UOE.
>>
>> With kind regards,
>>
>> Ivan
>>
>>
>>
>> On 6/25/18 3:06 PM, Stuart Marks wrote:
>>
>>> Hi all,
>>>
>>> Please review this small changeset that ensures that calling iterator()
>>> on an unmodifiable List (from List.of, et. al.) returns an instance that
>>> cannot be downcast to ListIterator and used as such.
>>>
>>> Bug:
>>>
>>>      https://bugs.openjdk.java.net/browse/JDK-8203670
>>>
>>> Webrev:
>>>
>>>      http://cr.openjdk.java.net/~smarks/reviews/8203670/webrev.0/
>>>
>>> Thanks,
>>>
>>> s'marks
>>>
>>>
>> --
>> With kind regards,
>> Ivan Gerasimov
>>
>>