RFR  Mistake in documentation of ArrayList#removeRange
ivan.gerasimov at oracle.com
Sat Mar 15 19:23:34 UTC 2014
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
> 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?
> 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
>>> 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
>>> 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.
More information about the core-libs-dev