RFR [8014066] Mistake in documentation of ArrayList#removeRange

[Martin Buchholz]

Many years ago, I wrote MOAT.java in an attempt to test many
implementations with one piece of code.  I would modify the list testing
part of that file to do something like

if (list instanceof AbstractList) {
  check that removeRange(size(), size()) is a no-op
  ... maybe check that removeRange with invalid indices does something
reasonable.
}

BUT: removeRange is not really a public API - it's an API for subclassers,
and so it is much more reasonable for removeRange to assume that it is
being called with valid indices.



On Sat, Mar 15, 2014 at 12:23 PM, Ivan Gerasimov
<ivan.gerasimov at oracle.com>wrote:

> Thank you Martin!
>
>
> On 15.03.2014 17:24, Martin Buchholz wrote:
>
>> We understand the intent of removeRange better now. I agree that making
>> further improvements is tricky.
>>
>> 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.
>>
> What about a regtest then?
> Wouldn't it be good to check at least that removeRange(size(), size())
> works, and removeRange(-1,size()) or (0, size()+1) throw the exception?
>
> Sincerely yours,
> Ivan
>
>
>  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
>>>
>>>
>>>
>>>
>>>
>>
>