Proof of concept for fluent bindings for ObservableValue

[Nir Lisker]

>
> In the PoC I made I specifically also disallowed 'null' as an input
>

I like the way ReactFX does it where the property is empty. I think that
this is also what you mean by disallowing `null` (in other contexts,
"disallowing null" would mean throwing an exception).

Not entirely sure what you mean by this.
>

Basically, what you said. My point was that this is a different API
section. The first deals with expanding the observables/properties methods.
The second with listeners methods. Even if mapping a property requires a
new listening model, like subscriptions, this is done under the hood.
Exposing this API should be a separate step. At least that's how I see it.

I'd be happy to spend more time and work on this. Perhaps it would be
> possible to collaborate on this?
>

That would be good. I will need to re-review the ReactFX internals and see
how your proposal differs exactly.

 By the way, do you make a distinction between ReactFX's Val and Var in
your proposal (one being read-only)?

On Sun, Apr 4, 2021 at 12:43 PM John Hendrikx <hjohn at xs4all.nl> wrote:

>
>
> On 02/04/2021 08:47, Nir Lisker wrote:
> > Hi John,
> >
> > I've had my eyes set on ReactFX enhancements for a while too, especially
> as
> > a replacement for the unsafe "select" mechanism. One of the things that
> > kept me from going forward with this is seeing what Valhalla will bring.
> > Generic specialization might save a lot of duplication work on something
> > like this, and Tomas touched another related issue [1], but since it
> could
> > be a long time before that happens, it's worth planning what we can
> extract
> > from ReactFX currently.
>
> Agreed, Valhalla is certainly a highly anticipated feature but I fear it
> is still a couple of years away.
>
> Even without any initial support for dealing with "? extends Number"
> from the various ObservableValue specializations I think looking into
> this can already be tremendous help.
>
> The proof of concept mainly requires you convert the Number to a
> suitable type when reading the property but has no problems in the other
> direction:
>
>      label.widthProperty().map(Number::doubleValue).map(x -> x + 1);
>
> Not pretty, but certainly workable. Specific methods could be introduced
> (even at a later time) to make this more streamlined, similar to what
> the Stream API offers with 'mapToDouble' etc.
>
> > I think that we should break the enhancements into parts.
> > The first that I would advise to look at are the additions to
> > properties/observables. Tomas had to create Val and Var because he
> couldn't
> > change the core interfaces, but we can. Fitting them with the Optional
> > methods like `isPresent`, `isEmpty`, `ifPresent`, `map`. `flatMap` etc.;
> > and `select` and friends, is already a good start that will address many
> > common requirements.
>
> Yes, Val/Var had to be created for that reason, and also because
> properties don't quite behave the same as streams -- streams with a
> "toBinding" method results in things people didn't quite expect.
>
> As far as the Optional methods go, I'm not entirely sure properties
> would benefit from all of them. Properties are not immutable like
> Optional and it may make less sense to fit them with 'isPresent',
> 'isEmpty' and 'ifPresent' ('ifPresent' would I think need to behave
> similar to 'addListener' or 'subscribe').
>
> In the PoC I made I specifically also disallowed 'null' as an input for
> functions like 'map' and 'flatMap' (opting to use 'orElse' semantics for
> 'null'), as this for allows much cleaner mapping (and especially flat
> mapping when selecting nested properties). If 'null' were to be allowed,
> I think at a minimum we'd need to add another method to allow for easy
> selecting of nested properties to avoid:
>
>      obs.flatMap(x -> x == null ? null : x.otherProperty())
>
> > The second part is related to listeners. The subscription model and event
> > streams try to solve the memory issues with hard and weak references, and
> > allow better composition.
>
> Not entirely sure what you mean by this. JavaFX's current model uses
> weak references which was I think an unfortunate decision as it can
> result in huge confusion.  For example, a direct binding will work, but
> with an indirection step a binding stops working:
>
>      button.textProperty()
>         .concat("World")  // weak binding used here
>         .addListener((obs, old, cur) -> System.out.println(cur));
>
> The above stops working, but without the 'concat' it keeps working.
>
> I think the use of weak listeners should be avoided and instead other
> mechanisms should be provided to make cleaning up easier. This is the
> main reason for 'conditionOn' and why ReactFX even had a specialized
> version of it: 'conditionOnShowing(Node)'.
>
> > The third part is for collections - things like transformation lists
> > (LiveList) and for other collections.
>
> This is indeed best saved for last. The problems there I think are less
> of an issue for now.
>
> > Since these share behavior under the hood, we need to look ahead, but in
> > terms of functionality, I think we should take smaller steps. It will
> also
> > be easier to propose these then.
>
> I've for this reason kept the PoC small with only the most basic
> functionality.  I did however add some work for a different subscription
> model, mainly because the internals of this code benefits greatly from
> it. It is however kept to a minimum.
>
> I'd be happy to spend more time and work on this. Perhaps it would be
> possible to collaborate on this?
>
> --John
>
> >
> > - Nir
> >
> > [1]
> >
> https://github.com/TomasMikula/ReactFX/wiki/Creating-a-Val-or-Var-Instance#the-javafx-propertynumber-implementation-issue
> >
> > On Wed, Mar 24, 2021 at 11:49 PM John Hendrikx <hjohn at xs4all.nl> wrote:
> >
> >> I just wanted to draw some attention to a recent proof of concept I made
> >> in this pull request: https://github.com/openjdk/jfx/pull/434
> >>
> >> It is based on the work I did in
> >> https://github.com/hjohn/hs.jfx.eventstream which is in part based on
> >> work done in ReactFX by Tomas Mikula. The PR itself however shares no
> >> code with ReactFX and is
> >> completely written by me.
> >>
> >> If there is interest, I'm willing to invest more time in smoothing out
> >> the API and documentation, investigating further how this would interact
> >> with the primitive types and adding unit test coverage (I have extensive
> >> tests, but thesea are written in JUnit 5, so they would require
> >> conversion or JavaFX could move to support JUnit 5).
> >>
> >> What follows below is the text of the PR for easy reading. Feedback is
> >> appreciated.
> >>
> >> ================
> >>
> >> This is a proof of concept of how fluent bindings could be introduced to
> >> JavaFX. The main benefit of fluent bindings are ease of use, type safety
> >> and less surprises. Features:
> >>
> >> Flexible Mappings
> >> Map the contents of a property any way you like with map, or map nested
> >> properties with flatMap.
> >>
> >> Lazy
> >> The bindings created are lazy, which means they are always invalid when
> >> not themselves observed. This allows for easier garbage collection (once
> >> the last observer is removed, a chain of bindings will stop observing
> >> their parents) and less listener management when dealing with nested
> >> properties. Furthermore, this allows inclusion of such bindings in
> >> classes such as Node without listeners being created when the binding
> >> itself is not used (this would allow for the inclusion of a
> >> treeShowingProperty in Node without creating excessive listeners, see
> >> this fix I did in an earlier PR: #185)
> >>
> >> Null Safe
> >> The map and flatMap methods are skipped, similar to java.util.Optional
> >> when the value they would be mapping is null. This makes mapping nested
> >> properties with flatMap trivial as the null case does not need to be
> >> taken into account in a chain like this:
> >>
> node.sceneProperty().flatMap(Scene::windowProperty).flatMap(Window::showingProperty).
> >>
> >> Instead a default can be provided with orElse or orElseGet.
> >>
> >> Conditional Bindings
> >> Bindings can be made conditional using the conditionOn method. A
> >> conditional binding retains its last value when its condition is false.
> >> Conditional bindings donot observe their source when the condition is
> >> false, allowing developers to automatically stop listening to properties
> >> when a certain condition is met. A major use of this feature is to have
> >> UI components that need to keep models updated which may outlive the UI
> >> conditionally update the long lived model only when the UI is showing.
> >>
> >> Some examples:
> >>
> >> void mapProperty() {
> >>    // Standard JavaFX:
> >>    label.textProperty().bind(Bindings.createStringBinding(() ->
> >> text.getValueSafe().toUpperCase(), text));
> >>
> >>    // Fluent: much more compact, no need to handle null
> >>    label.textProperty().bind(text.map(String::toUpperCase));
> >> }
> >>
> >> void calculateCharactersLeft() {
> >>    // Standard JavaFX:
> >>
> >>
> label.textProperty().bind(text.length().negate().add(100).asString().concat("
> >>
> >> characters left"));
> >>
> >>    // Fluent: slightly more compact and more clear (no negate needed)
> >>    label.textProperty().bind(text.orElse("").map(v -> 100 - v.length() +
> >> " characters left"));
> >> }
> >>
> >> void mapNestedValue() {
> >>    // Standard JavaFX:
> >>    label.textProperty().bind(Bindings.createStringBinding(
> >>      () -> employee.get() == null ? ""
> >>          : employee.get().getCompany() == null ? ""
> >>          : employee.get().getCompany().getName(),
> >>      employee
> >>    ));
> >>
> >>    // Fluent: no need to handle nulls everywhere
> >>    label.textProperty().bind(
> >>      employee.map(Employee::getCompany)
> >>              .map(Company::getName)
> >>              .orElse("")
> >>    );
> >> }
> >>
> >> void mapNestedProperty() {
> >>    // Standard JavaFX:
> >>    label.textProperty().bind(
> >>      Bindings.when(Bindings.selectBoolean(label.sceneProperty(),
> >> "window", "showing"))
> >>        .then("Visible")
> >>        .otherwise("Not Visible")
> >>    );
> >>
> >>    // Fluent: type safe
> >>    label.textProperty().bind(label.sceneProperty()
> >>      .flatMap(Scene::windowProperty)
> >>      .flatMap(Window::showingProperty)
> >>      .orElse(false)
> >>      .map(showing -> showing ? "Visible" : "Not Visible")
> >>    );
> >> }
> >>
> >> void updateLongLivedModelWhileAvoidingMemoryLeaks() {
> >>    // Standard JavaFX: naive, memory leak; UI won't get garbage
> collected
> >>    listView.getSelectionModel().selectedItemProperty().addListener(
> >>      (obs, old, current) ->
> >> longLivedModel.lastSelectedProperty().set(current)
> >>    );
> >>
> >>    // Standard JavaFX: no leak, but stops updating after a while
> >>    listView.getSelectionModel().selectedItemProperty().addListener(
> >>      new WeakChangeListener<>(
> >>        (obs, old, current) ->
> >> longLivedModel.lastSelectedProperty().set(current)
> >>      )
> >>    );
> >>
> >>    // Standard JavaFX: fixed version
> >>    listenerReference = (obs, old, current) ->
> >> longLivedModel.lastSelectedProperty().set(current);
> >>
> >>    listView.getSelectionModel().selectedItemProperty().addListener(
> >>      new WeakChangeListener<>(listenerReference)
> >>    );
> >>
> >>    // Fluent: naive, memory leak... fluent won't solve this...
> >>    listView.getSelectionModel().selectedItemProperty()
> >>        .subscribe(longLivedModel.lastSelectedProperty()::set);
> >>
> >>    // Fluent: conditional update when control visible
> >>
> >>    // Create a property which is only true when the UI is visible:
> >>    ObservableValue<Boolean> showing = listView.sceneProperty()
> >>        .flatMap(Scene::windowProperty)
> >>        .flatMap(Window::showingProperty)
> >>        .orElse(false);
> >>
> >>    // Use showing property to automatically disconnect long lived model
> >>    // allowing garbage collection of the UI:
> >>    listView.getSelectionModel().selectedItemProperty()
> >>      .conditionOn(showing)
> >>      .subscribe(longLivedModel.lastSelectedProperty()::set);
> >>
> >>    // Note that the 'showing' property can be provided in multiple ways:
> >>    // - create manually (can be re-used for multiple bindings though)
> >>    // - create with a helper: Nodes.showing(Node node) ->
> >> ObservableValue<Boolean>
> >>    // - make it part of the Node class; as the fluent bindings only bind
> >> themselves
> >>    //   to their source when needed (lazy binding), this won't create
> >> overhead
> >>    //   for each node in the scene
> >> }
> >> Note that this is based on ideas in ReactFX and my own experiments in
> >> https://github.com/hjohn/hs.jfx.eventstream. I've come to the
> conclusion
> >> that this is much better directly integrated into JavaFX, and I'm hoping
> >> this proof of concept will be able to move such an effort forward.
> >>
> >> --John
> >>
> >
>