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

Roger Riggs Roger.Riggs at Oracle.com
Fri Mar 14 11:10:36 UTC 2014

Hi,

Without getting into the details, make sure that the behavior seen by applications is not changed without careful and exhaustive review. Usually, the specification is updated to match the existing long standing behavior including exceptions and edge cases.

Roger

On 3/14/14 6:50 AM, Ivan Gerasimov wrote:

Thanks David!

On 14.03.2014 12:31, David Holmes wrote:

Second point is that the condition "fromIndex >= size()" is already captured by the condition "if {@code fromIndex} or {@code toIndex} is out of range". By definition fromIndex is out-of-range if it is < 0 or_ _>= size(). So I don't see any error here even if there is some redundancy. I don't think that 'fromIndex < size()' restriction is quite natural._ _Seems perfectly natural to ensure the fromIndex is somewhere in the_ _available range._ _Sorry, what I meant was that it is too restrictive._ _'fromIndex <= size()' is indeed natural._ _And finally, the AbstractList.removeRange change:_ _+ * @throws NoSuchElementException if {@code (toIndex > size())} seems to reflect an implementation accident rather than a clear API design choice. If the toIndex is out of range then IndexOutOfBoundsException is what should be thrown. Otherwise you constrain any subtypes that override this to throw an exception type that only has meaning with the AbstractList implementation strategy - and by doing so you are making ArrayList.removeRange violate this new specification. It is unfortunate that the existing specification(s) for removeRange are lacking this key detail on exception processing, and lack consistency between AbstractList and it's sublcass ArrayList. I think ArrayList has the better/cleaner specification and that should be pushed up AbstractList and AbstractList's implementation should be modified to explicitly check the pre-conditions. Again, I agree with you. One thing I noticed is that some methods I mentioned above (List.subList(), Arrays.sort(), etc) throw IllegalArgumentException when fromIndex > toIndex, not IndexOutOfBoundException. Wouldn't it be more correct to adopt this into removeRange() too? Unfortunately there are a lot of inconsistencies in the API design throughout the JDK - even in classes that are somewhat related. That said List.subList throws IOOBE, But I do see examples of: IllegalArgumentException - if fromIndex > toIndex which is definitely more correct if both values are in range, but then what if both are out of range? Anyway you can't just arbitrarily make changes to the specifications of these methods - even if trying to fix obvious inconsistencies. The main priority is that specs and implementations agree; and that specs in subtypes are consistent with the spec in the supertypes. I see a number of issues here that may be tricky to reconcile having regard for compatability. What we have how is: AbstractList#removeRange() - spec misses throws section; - implementation has a bug: the function may throw NoSuchElementException instead of IndexOutOfBoundsException; ArrayList#removeRange() - spec contains wrong condition 'fromIndex >= size()' ; - implementation has a bug: the condition fromIndex <= toIndex isn't_ _checked at all._ _Thus, we'll need to adjust both spec and implementation of both base_ _and derived classes._ _That's why I ask, if it may make sense to add IllegalArgumentException_ _for fromIndex > toIndex case, since the spec needs to be corrected anyway. Of course, no problem to leave only IndexOutOfBoundsException. Concerning the ambiguity, which exception to throw if toIndex < fromIndex and some of them is out of range. I believe, the conditions should be checked in the order they're listed in the spec. Therefore, IndexOutOfBoundsException in this case. Sincerely yours, Ivan

More information about the core-libs-dev mailing list