RFR [8014066] Mistake in documentation of ArrayList#removeRange

[David Holmes]

Hi Martin,

On 15/03/2014 11:24 PM, Martin Buchholz wrote:
> We understand the intent of removeRange better now. I agree that making
> further improvements is tricky.

Almost impossible I'd say :)

> I think we can all agree that the initial patch
> http://cr.openjdk.java.net/~igerasim/8014066/0/webrev/jdk.patch
> is a clear improvement (simple spec bug fix)
> fromIndex == size  must be OK because it results from a call to clear on an
> already empty list.
> Maybe we should check that unambitious fix in first.

I agree. Though I'd suggest that in keeping with the "fixing a typo" 
theme we simply change:

    *          fromIndex >= size() ||

to

     *          fromIndex > size() ||

Although this constraint is already implicit in the "in range" 
requirement, a one character change is much easier to accept as correct.

David
-----

> Changing the spec for removeRange to add more requirements on subclass
> implementations seems wrong because there are existing implementations and
> because the (underspecified) contract is that removeRange will only be
> called from a call to clear, which in turn will ensure that the indices are
> legal. Adding more bounds checks now will just add overhead without
> catching user mistakes in practice.
>
> Improving the spec to clarify the contract as originally designed by Josh
> would be good, but it will be hard to find good wording and get approval
> from all the interested parties.
>
> Software is hard.
>
>
> On Fri, Mar 14, 2014 at 4:42 AM, Doug Lea <dl at cs.oswego.edu> wrote:
>
>> On 03/14/2014 02:38 AM, Martin Buchholz wrote:
>>
>>> On Thu, Mar 13, 2014 at 3:59 PM, Doug Lea <dl at cs.oswego.edu
>>> <mailto:dl at cs.oswego.edu>> wrote:
>>>      On 03/13/2014 12:34 PM, Martin Buchholz wrote:
>>>          I notice there are zero jtreg tests for removeRange.  That should
>>> be fixed.
>>>
>>>          I notice there is a removeRange in CopyOnWriteArrayList, but it is
>>>          package-private instead of "protected", which seems like an
>>> oversight.
>>>            Can Doug
>>>          remember any history on that?
>>>
>>>
>>>      CopyOnWriteArrayList does not extend AbstractList, but its
>>>      sublist does. The sublist relies on COWAL.removeRange only for clear.
>>>      So COWAL sublist clearing uses the same idea as
>>>      AbstractList, and gives it the same name, but it is not the
>>>      same AbstractList method, so need not be protected.
>>>
>>>
>>> Ahh OK, I think the party line for *users* is if they want to remove a
>>> range of
>>> elements from a list, use list.subList(fromIndex, toIndex).clear (), so
>>> there's
>>> no advantage in making COWAL.removeRange a public interface.
>>>
>>
>> Right. This relates to the question of range checks for removeRange.
>> Josh Block created removeRange as part of an incompletely documented
>> recipe for AbstractList-based List implementations. It was intended that
>> people creating new kinds of Lists implement this as a helper
>> method to simplify sublist implementations. It is designed
>> to be called only from other public methods that would either themselves
>> perform range checks or skip them if statically unnecessary.
>> So, there aren't any redundant checks in the default removeRange.
>> Leaving this partially exposed as "protected" doesn't quite
>> ensure that implementors follow this guidance, but in practice,
>> I suspect that they all have. So it is not clear whether changing
>> the spec or the implementation would be doing anyone a favor.
>>
>> -Doug
>>
>>
>>
>>