On Tue, 26 Mar 2024 16:32:53 GMT, Karthik P K <k...@openjdk.org> wrote:
>> drmarmac has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Remove outdated comment
>
> modules/javafx.controls/src/main/java/javafx/scene/control/ControlUtils.java
> line 176:
>
>> 174: .distinct()
>> 175: .filter(removeRowFilter)
>> 176: .forEach(row -> {
>
> In the java.util.stream package
> [docs](https://docs.oracle.com/en/java/javase/16/docs/api/java.base/java/util/stream/package-summary.html#SideEffects)
> it is mentioned that `forEach()` method operates only via side-effects. So
> do you think we should avoid using `forEach()` here and iterate the generated
> list separately to clear selected index?
I'd say .forEach() is used correctly here, according to docs, it guarantees
execution of the side-effects (add to removed list & clear index), just not in
any particular order. This way we avoid multiple iteration.
> modules/javafx.controls/src/main/java/javafx/scene/control/ControlUtils.java
> line 185:
>
>> 183: .mapToInt(TablePositionBase::getRow)
>> 184: .distinct()
>> 185: .forEach(row -> {
>
> Similar comment as above. Here if we do not use `forEach()` on streams, we
> can also avoid using array of size one for keeping count as well right?
We'd need to iterate twice as well (select index & count), with forEach it's
just once.
-------------
PR Review Comment: https://git.openjdk.org/jfx/pull/1430#discussion_r1540712652
PR Review Comment: https://git.openjdk.org/jfx/pull/1430#discussion_r1540712756
