RFR  Mistake in documentation of ArrayList#removeRange
david.holmes at oracle.com
Sun Mar 16 04:51:55 UTC 2014
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
> 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() ||
* fromIndex > size() ||
Although this constraint is already implicit in the "in range"
requirement, a one character change is much easier to accept as correct.
> 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