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

List:       openjdk-openjfx-dev
Subject:    Re: RFR: 8274771: Map, FlatMap and OrElse fluent bindings for ObservableValue [v14]
From:       John Hendrikx <jhendrikx () openjdk ! java ! net>
Date:       2022-05-28 7:26:49
Message-ID: NcelwVoaaky1Ml85xEH8d3HWmKkeV7h-4k8F4hDiTLY=.21d6256d-3f18-4dff-b9aa-3eed5877be69 () github ! com
[Download RAW message or body]

On Fri, 27 May 2022 23:31:42 GMT, Kevin Rushforth <kcr@openjdk.org> wrote:

> I reviewed the public API changes, and this looks like a great addition to JavaFX \
> bindings. I think there might be time to get this into JavaFX 19, presuming that \
> there are no issues with the testing or implementation, so let's proceed down that \
> path. 
> I left one comment on the API docs as well as pointed out the public methods that \
> will need an `@since 19` javadoc tag. 
> Once that is updated you can propagate the javadoc changes to the CSR (including \
> the `@since` tags) and move it to "Proposed". I'll formally review it later, once \
> the code review is closer to being done.

Thanks, I've made the changes and updated the CSR with the latest docs. It's now \
proposed.

> modules/javafx.base/src/main/java/javafx/beans/value/ObservableValue.java line 205:
> 
> > 203:      * resulting value is {@code null}. If the mapping resulted in {@code \
> >                 null}, then the
> > 204:      * resulting value is also {@code null}.
> > 205:      * <p>
> 
> It might be worth borrowing some language from `Optional::flatMap`. Maybe something \
> like this? 
> 
> This method is similar to {@link #map(Function)}, but the mapping function is
> one whose result is already an ObservableValue, and if invoked, flatMap does
> not wrap it within an additional ObservableValue.

Added this and put code tags where necessary.

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

PR: https://git.openjdk.java.net/jfx/pull/675


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

Configure | About | News | Add a list | Sponsored by KoreLogic