RFR [8014066] Mistake in documentation of ArrayList#removeRange (original) (raw)

David Holmes david.holmes at oracle.com
Sun Mar 16 04:51:55 UTC 2014

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

More information about the core-libs-dev mailing list