RFR [8014066] Mistake in documentation of ArrayList#removeRange
Ivan Gerasimov
ivan.gerasimov at oracle.com
Thu Mar 13 19:03:25 UTC 2014
Would you please take a look at the updated webrev:
http://cr.openjdk.java.net/~igerasim/8014066/1/webrev/
In addition to the javadoc correction, it includes a regression test and
a check for (fromIndex > toIndex).
The last check turns out to be sufficient for removeRange() method to
agree with the javadoc.
Other checks are handled by the following System.arraycopy() call.
Sincerely yours,
Ivan
On 13.03.2014 22:27, Ivan Gerasimov wrote:
>
> On 13.03.2014 22:16, Ivan Gerasimov wrote:
>>
>> On 13.03.2014 20:37, Martin Buchholz wrote:
>>> The corner case for removeRange(SIZE, SIZE) does seem rather tricky,
>>> and it's easy for an implementation to get it wrong. All the more
>>> reason to add tests!
>>>
>> It was a good idea to add a test for removeRange().
>> I just discovered that IOOB isn't thrown when you call removeRange(1,
>> 0) or removeRange(4, 0).
>> However, the exception is thrown when you call removeRange(5, 0).
>>
>> The fix seems to become a bit more than just a doc typo fix :-)
>>
>
> And when you do list.removeRange(1, 0), the list becomes longer.
>
>
>> Sincerely yours,
>> Ivan
>>
>>>
>>> On Thu, Mar 13, 2014 at 9:15 AM, Ivan Gerasimov
>>> <ivan.gerasimov at oracle.com <mailto:ivan.gerasimov at oracle.com>> wrote:
>>>
>>> Thank you Chris!
>>>
>>> It is System.arraycopy() method, where checking is performed and
>>> the exception is thrown.
>>> Here's this code:
>>> if ( (((unsigned int) length + (unsigned int) src_pos) >
>>> (unsigned int) s->length())
>>> || (((unsigned int) length + (unsigned int) dst_pos) >
>>> (unsigned int) d->length()) ) {
>>> THROW(vmSymbols::java_lang_ArrayIndexOutOfBoundsException());
>>> }
>>>
>>> This confirms that size() is a valid value for fromIndex.
>>>
>>> Another way to think of it is that fromIndex <= toIndex, and
>>> toIndex can be equal to size().
>>> Therefore, fromIndex can be equal to size() too.
>>>
>>> The documentation also says that 'If toIndex==fromIndex, this
>>> operation has no effect.', so removeRange(size(), size()) is a
>>> valid expression and should not cause an exception be thrown (and
>>> it actually does not).
>>>
>>> Sincerely yours,
>>> Ivan
>>>
>>>
>>> On 13.03.2014 19:47, Chris Hegarty wrote:
>>>
>>> Ivan,
>>>
>>> This does look a little odd, but since fromIndex is inclusive
>>> I would think that it should throw if passed a value of
>>> size() ??
>>>
>>> -Chris.
>>>
>>> On 13 Mar 2014, at 15:29, Ivan Gerasimov
>>> <ivan.gerasimov at oracle.com <mailto:ivan.gerasimov at oracle.com>>
>>> wrote:
>>>
>>> Hello!
>>>
>>> Would you please review a simple fix of the javadoc for
>>> ArrayList#removeRange() method?
>>>
>>> The doc says that IndexOutOfBoundsException is thrown if
>>> fromIndex or toIndex is out of range (fromIndex < 0 ||
>>> fromIndex >= size() || toIndex > size() || toIndex <
>>> fromIndex).
>>>
>>> The condition 'fromIndex >= size()' isn't true and should
>>> be removed from the doc.
>>>
>>> For example, the code list.removeRange(size(), size())
>>> does not throw any exception.
>>>
>>> BUGURL: https://bugs.openjdk.java.net/browse/JDK-8014066
>>> WEBREV:
>>> http://cr.openjdk.java.net/~igerasim/8014066/0/webrev/
>>> <http://cr.openjdk.java.net/%7Eigerasim/8014066/0/webrev/>
>>>
>>> Sincerely yours,
>>> Ivan
>>>
>>>
>>>
>>>
>>>
>>
>>
>>
>
>
>
More information about the core-libs-dev
mailing list