On Wed, 27 Mar 2024 12:23:47 GMT, Marius Hanl <mh...@openjdk.org> wrote:

>> 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.
>
> Another idea is to use `toList()`, which is a very efficient operation and 
> then iterate over it.

> 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?

`forEach` is used correctly here. From the docs:
> With the exception of terminal operations 
> [forEach](https://docs.oracle.com/en/java/javase/21/docs/api/java.base/java/util/stream/Stream.html#forEach(java.util.function.Consumer))
>  and 
> [forEachOrdered](https://docs.oracle.com/en/java/javase/21/docs/api/java.base/java/util/stream/Stream.html#forEachOrdered(java.util.function.Consumer)),
>  side-effects of behavioral parameters may not always be executed when the 
> stream implementation can optimize away the execution of behavioral 
> parameters without affecting the result of the computation.

> Another idea is to use `toList()`, which is a very efficient operation and 
> then iterate over it.

That's still 2 iterations. If the code is not performance-critical it doesn't 
matter.

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

PR Review Comment: https://git.openjdk.org/jfx/pull/1430#discussion_r1541140317

Reply via email to