>
> 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 <hj...@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 <hj...@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
> >>
> >
>
