RFR [8014066] Mistake in documentation of ArrayList#removeRange
Ivan Gerasimov
ivan.gerasimov at oracle.com
Thu Mar 13 19:42:48 UTC 2014
Sorry, I forgot to incorporate changes to AbstractList.removeRange()
documentation, which you Martin had suggested.
Here's the webrev with those included:
http://cr.openjdk.java.net/~igerasim/8014066/2/webrev/
Sincerely yours,
Ivan
On 13.03.2014 23:03, Ivan Gerasimov wrote:
> 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