[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-openjfx-dev
Subject:    Re: RFR: 8271865: SortedList::getViewIndex behaves not correctly for some index values
From:       Kevin Rushforth <kcr () openjdk ! org>
Date:       2024-04-30 0:58:11
Message-ID: o9vH93StbWBfz3Io7EYXYpPKW2EKPmAt8YdPcCjmO8k=.041a0d54-ce2e-4a5a-8514-0b142fea3aef () github ! com
[Download RAW message or body]

On Sun, 24 Mar 2024 15:11:29 GMT, drmarmac <duke@openjdk.org> wrote:

> This PR adds the missing checks, as well as code documentation that an \
> IndexOutOfBoundsException may be thrown.

The fix and tests look good. I left a couple comments on the docs.

modules/javafx.base/src/main/java/javafx/collections/transformation/TransformationList.java \
line 121:

> 119:      * @param index the index in this list
> 120:      * @return the index of the element's origin in the source list
> 121:      * @throws IndexOutOfBoundsException if the index is out of range (index \
> &lt; 0 || index >= size())

Suggestion: consider using code case for the variables and equation?

modules/javafx.base/src/main/java/javafx/collections/transformation/TransformationList.java \
line 134:

> 132:      * @param index the index of an element in this list
> 133:      * @return the index of the element's origin in the provided list
> 134:      * @throws IndexOutOfBoundsException if the index is out of range (index \
> &lt; 0 || index >= list.getSource().size())

There is no`getSource` method in `ObservableList`. That should be `... index >= \
size())`

-------------

PR Review: https://git.openjdk.org/jfx/pull/1432#pullrequestreview-2029789757
PR Review Comment: https://git.openjdk.org/jfx/pull/1432#discussion_r1583998595
PR Review Comment: https://git.openjdk.org/jfx/pull/1432#discussion_r1583874591


[prev in list] [next in list] [prev in thread] [next in thread]