Proof of concept for fluent bindings for ObservableValue

[John Hendrikx]

Nir,

I've created a new branch which contains all the changes which we've 
discussed so far.  It contains JUnit 4 tests (in backported form), and a 
reduced API.

However, the changes made in the sandbox did not fully compile due to 
package restrictions, and I had to make a bit more API public.

The issue is that ObervableValue and ObjectBinding are in two different 
packages.  LazyObjectBinding must subclass ObjectBinding and requires 
two new methods (isObserved and allowInvalidation). ObservableValue uses 
classes like MappedBinding which subclass LazyObjectBinding.

Making the **Binding classes package private means ObservableValue can't 
access them. Moving the binding classes to ObservableValue's package 
means that LazyObjectBinding cannot access the isObserved and 
allowInvalidation methods of ObjectBinding.

So, we have two choices (that I can see):

1) Make LazyObjectBinding and subclasses public so they can be accessed 
from ObservableValue's package. LazyObjectBinding still must be in same 
package as ObjectBinding since the isObserved and allowInvalidation 
methods are package private.

2) Make isObserved and allowInvalidation *protected* methods of 
ObjectBinding so that LazyObjectBinding can access them from 
ObservableValue's package (also move all the other new binding classes 
there).

I went for the second option in my implementation as this exposes the 
least additional API.  I also think that the additions to ObjectBinding 
are relatively harmless, and in the case of isObserved might actually be 
useful for debugging.

If we ever want to make LazyObjectBinding and its subclasses public, 
we'd have to move them to the binding package as they make more sense 
there, but as long as they're package private they can safely live in 
the same package as ObservableValue.

Please see here for a draft version. If you could do a quick check and 
you're agreed with the route I took to resolve the package access 
restrictions, I can submit this as a PR:

https://github.com/hjohn/jfx/tree/feature/fluent-bindings

--John

On 24/03/2021 22:49, John Hendrikx 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
>