>
> Sorry, I'm not quite sure what you mean by this. Could you elaborate?
> The methods orElse and orElseGet are present in the PoC, and I think
> they're highly useful to have to deal with nulls.
The methods that you call orElse and orElseGet return an
ObservableValue<T>. The Optional methods with the same names return the
wrapped value (of type T). For comparison, ReactFX has:
T getOrElse(T defaultValue)
T getOrSupply(Supplier<? extends T> defaultSupplier)
Val<T> orElseConst(T other)
Val<T> orElse(ObservableValue<T> other)
So it deals with both getting the wrapped value in null cases and with
getting a "dynamic value" in null cases. I find the Optional-like method 'T
getOrElse(T defaultValue)' useful (along with others such as ifPresent
because Optional is just useful for dealing with wrapped values). What I'm
saying is that we should be careful with the names if we include both
"constant" and "dynamic" versions of 'orElse'-like methods.
The null check in ReactFX's #computeValue is
> actually checking the result of the mapping function, not whether the
> function instance itself was null.
>
I didn't mean the null-ness of the map argument. What I meant was that
there is this implementation, which is similar to what ReactFX does:
public static <T, U> ObservableValue<U> mapping(ObservableValue<T>
source, Function<? super T, ? extends U> mapper) {
return nullableMapping(source, mapper.apply(v)); // don't deal with a
null v here
}
public static <T, U> ObservableValue<U>
nullableMapping(ObservableValue<T> source, Function<? super T, ? extends U>
mapper) {
return new LazyObjectBinding<>() {
...
@Override
protected U computeValue() {
T value = source.getValue();
return value == null ? null ? mapper.apply(value); // do it here
instead
}
};
}
---
As for my points 3 and 4, I'll have to try and play with the implementation
myself, which will be easier to do when there is some PR in the works.
To close "Bindings.select*(): add new type-safe template based API
> instead of legacy-style set of methods" we'd need the flatMap/select
> method to be included.
Yes. I think that we can include flatMap in this iteration, perhaps as a
separate PR?
I don't think we really need specialized methods for primitives (or at
> least, not right away). At this point the primitive versions only
> really differ in what value they'd return if the binding would be null
> and perhaps they do a little less boxing/unboxing. Since you can select
> the default value with #orElse which is more flexible. I don't see much
> use to add those variants.
I agree, I would avoid bloating the primitive specialization classes for
now, especially when Valhalla is planned to take care of those.
The ticket is a bit unclear as I can't see what type "x" is.
>
Yes, but I got the impression that either way it can be solved with map (or
flatMap).
On Tue, Sep 14, 2021 at 8:44 AM John Hendrikx <hj...@xs4all.nl> wrote:
>
>
> On 14/09/2021 03:14, Nir Lisker wrote:
> > Sounds good.
> >
> > Some points I have (maybe some are premature):
> >
> > 1. I still think that adding the Optional methods for orElse and
> orElseGet
> > could be useful. Unless I can be convinced otherwise, I suggest that we
> be
> > careful with the naming of current methods that return a binding.
>
> Sorry, I'm not quite sure what you mean by this. Could you elaborate?
> The methods orElse and orElseGet are present in the PoC, and I think
> they're highly useful to have to deal with nulls.
>
> > 2. I see that in ReactFX the Val.map will pass to MappedVal the mapping
> > function as-is, and the null check is done in computeValue(). In your
> > implementation, the LazyBinding (equivalent of MappedVal) is passed a
> > composite mapping that deals with null and the computeValue() can just
> use
> > that new mapping function. I think that the end behavior is the same, but
> > does your way use more memory for the extra Function lambda?
>
> I think you may have misinterpreted what happens here. I'm ensuring that
> the mapping function itself isn't null (fail fast) by using
> Objects.requireNonNull -- it doesn't produce a new lambda. ReactFX
> doesn't do this check and will throw a NPE when the mapping function is
> used for the first time. The null check in ReactFX's #computeValue is
> actually checking the result of the mapping function, not whether the
> function instance itself was null.
>
> > 3. Why does nullableMapping creates an anonymous subtype of LazyBinding,
> > while flatMapping creates a concrete instance of FlatMapBinding?
>
> I suppose I felt it was small enough to inline, but there's no
> particular reason to write it one way or the other. The nullableMapping
> can be extracted to a class to make it more consistent.
>
> > 4. Why does orElseGet call Bindings.nullableMapping directly, while map
> > calls Bindings.mapping which in turn calls Bindings.nullableMapping?
>
> It's because of the null check. For mapping I could wrap the function
> parameter in Objects.requireNonNull as it will be evaluated immediately.
> For orElseGet, I had to put the check on a separate line (as the first
> use of supplier parameter is inside a lambda).
>
> I prefer to do null checks as early as possible (so you get a better
> stack trace) but I also prefer to write interface default
> implementations as short as possible. I could write #map to look like
> #orElseGet but it would become one line longer, or I could write
> #orElseGet to do a call to a separate Bindings method and move the null
> check there but then the stack trace for the NPE would no longer point
> to the #orElseGet interface method as first trace line.
>
> I can rewrite this however if you would like, but that's sort of the
> reasoning I had I think when I wrote this code balancing short default
> implementations vs delayed null checks.
>
> If you would want it more consistent, then I think I'd prefer to write
> #map like #orElseGet.
>
> > 5. I noticed also that subscribeInvalidations in ObservableValue will
> need
> > to be hidden.
>
> Yes, that should be no problem. We can either fall back to using
> standard listener management, or create a small helper that works with
> the subscription model.
>
> > Some related JBS issues that I found that we might be able to use (or at
> > least close at some point):
> > https://bugs.openjdk.java.net/browse/JDK-8091544
>
> To close "Bindings.select*(): add new type-safe template based API
> instead of legacy-style set of methods" we'd need the flatMap/select
> method to be included.
>
> I don't think we really need specialized methods for primitives (or at
> least, not right away). At this point the primitive versions only
> really differ in what value they'd return if the binding would be null
> and perhaps they do a little less boxing/unboxing. Since you can select
> the default value with #orElse which is more flexible I don't see much
> use to add those variants.
>
> > https://bugs.openjdk.java.net/browse/JDK-8091316
>
> "Add a Bindings.map() method".
>
> The ticket is a bit unclear as I can't see what type "x" is.
>
> So this seems to be for a use case where an Observable "x" should be
> bound with some transformation to the windowTitleProperty. I'm assuming
> x is an Observable because it is used in the list of dependencies.
>
> If x is only an Observable, then I don't think the PoC will help.
>
> If x is more (it is hard to tell from the ticket) you might be able to
> map it. However, x seems to have a "getY()" method so it doesn't look
> like it is a Property or ObservableValue...
>
> Making some assumptions here and assuming x has an y property itself,
> you could write this as:
>
> windowsTitleProperty.bind(x.flatMap(XClass::yProperty).map(y -> {
> switch(y) {
> case ...
> }
> });
>
> Or:
>
> windowsTitleProperty.bind(x.map(x2 -> {
> switch(x2.getY()) {
> case ...
> }
> });
>
> Or perhaps x is the correct property already, then it be:
>
> windowsTitleProperty.bind(x.map(y -> {
> switch(y) {
> case ...
> }
> });
>
> --John
>
> > On Mon, Sep 13, 2021 at 3:05 AM John Hendrikx <hj...@xs4all.nl> wrote:
> >
> >>
> >>
> >> On 12/09/2021 02:05, Nir Lisker wrote:
> >>> I've gotten back to look at this.
> >>>
> >>> For now I'm dealing only with the nullableMapping method in Bindings so
> >>> we can limit the amount of new classes to LazyObjectBinding
> >>> (FlatMapBinding and ConditionalBinding can come later). This method is
> >>> used by map, orElse and orElseGet in ObservableValue. Of these, map is
> >>> the only fundamental one since the other 2 can be represented by it. I
> >>> don't mind keeping them in the discussion, though I will be centered on
> >>> the map method.
> >>>
> >>> The implementation of these methods rely on Bindings,
> LazyObjectBinding,
> >>> and Subscription in the current implementation. I think that we can
> >>> introduce these internally for now. The biggest hurdle left are the
> >>> public changes to ObjectBinding. If we add protected methods, we need
> to
> >>> be sure that by the end of this large task they would have been the
> >>> right ones to add and at the right place. This is why I recommend
> adding
> >>> them at the package visibility level and add LazyObjectBinding (and
> >>> friends) in its package so they can extend it. I understand that this
> >>> can look ugly, but moving internal implementation is cheap, and in this
> >>> case, since the coupling involves about 3 classes, is very cheap. This
> >>> will lower the initial integration barrier and let the community get
> >>> used to- and give feedback on the new changes.
> >>
> >> I think that's a good idea, there is no direct need to make those
> >> protected methods part of the public API as the usefulness of those
> >> methods will be limited and the major use case will basically be
> >> provided by LazyObjectBinding already.
> >>
> >>> This will leave only 1 change that we are committed to, and that's the
> >>> new API on ObservableValue (which is the map method in this case). The
> >>> method looks good; the only question, which has arisen in a few places,
> >>> is how to handle null. As we discussed here, this method works like its
> >>> ReactFX counterpart, ignoring null. My questions would be:
> >>> 1. Is there a good reason to allow null? If so, do we add a new method
> >>> for it, or do we pass some parameter to the current method to indicate
> >> that?
> >>
> >> In JavaFX, null is something we have to deal in some fashion as
> >> properties can easily be null. For the "primitive" properties, null (if
> >> encountered) is translated to a default value. For StringProperty it
> >> could be an empty string although JavaFX doesn't do this. For
> >> ObjectProperty there is no sensible default possible.
> >>
> >> In ReactFX, nulls are indeed skipped when mapping as it considers null
> >> to be an empty value, and empty values are skipped according to the
> >> documentation. The code below will not throw an NPE in the mapping
> >> function and will simply result in null:
> >>
> >> Var.newSimpleVar(null).map(x -> x + "2").getValue();
> >>
> >> This is similar to the PoC implementation:
> >>
> >> new SimpleStringProperty().map(x -> x + "2").getValue());
> >>
> >> Having worked with ReactFX and also the PoC, I think it would be very
> >> cumbersome to have to deal with nulls in mapping functions, as many
> >> simple mappings expressed with a short lambda would need to deal with
> >> the null case with a ternary or an if/else block.
> >>
> >> In the PoC any mapping you could need that requires mapping null
> >> explicitely can be expressed in another form:
> >>
> >> .map(x -> x == null ? "empty!" : x + "2")
> >>
> >> becomes:
> >>
> >> .map(x -> x + "2").orElse("empty!)
> >>
> >> which is not only more concise, but allows to delay dealing with null
> >> until the very end:
> >>
> >> .map(x -> x == null ? "empty!" : fetchDataWhichCouldBeNull(x))
> >> .map(x -> x == null ? "empty!" : x + "2")
> >>
> >> versus:
> >>
> >> .map(Helper::fetchDataThatCouldBeNull)
> >> .map(x -> x + "2")
> >> .orElse("empty!")
> >>
> >> You don't have to delay it though, if for some reason you would want to
> >> map the 2nd null case differently, you could use "orElse" after each
> >> mapping still.
> >>
> >> Although for mapping this may seem somewhat contrived, for selecting (or
> >> flatMapping) where you go through a chain of properties (like
> >> Node->Scene->Window) being able to delay dealing with nulls leads to
> >> more concise and IMHO more expressive code.
> >>
> >> So in summary, and to answer your first question, I don't think there is
> >> a good reason to allow null to be passed to mapping functions. We do
> >> need to deal with nulls though, and that's what "orElse" is for. This
> >> could also be done with an additional parameter to "map" (a
> >> "mapOrDefault" similar to "getOrDefault" from the collections API) but I
> >> think we'd be better served with multiple methods that take a single
> >> argument as the resulting code is easier to understand especially when
> >> one of the arguments is a lambda.
> >>
> >>
> >>> 2. If we want to replace the Bindings.select (non-type safe) API, can
> we
> >>> do it with our current way of treating null?
> >>
> >> In the current Bindings.select API, null is skipped when encountered and
> >> the resulting value of the chain will be null. This is exactly what
> >> "flatMap" in the PoC does as well, in other words:
> >>
> >> Bindings.select(nodeProperty, "scene", "window", "showing")
> >>
> >> is exactly equivalent to:
> >>
> >> nodeProperty.flatMap(Node::sceneProperty)
> >> .flatMap(Scene::windowProperty)
> >> .flatMap(Window::showingProperty)
> >>
> >> No null checks are needed, and the binding will be null if any of the
> >> selected properties contain null. Note that both versions return null
> >> despite the fact that the last property selected is a primitive boolean
> >> which normally cannot hold null.
> >>
> >> However, Bindings also offers selectBoolean. In this case it does
> >> indeed return false when any of the properties contains null, but it
> >> also logs this uncomfortable warning then:
> >>
> >> WARNING: Value of select-binding has wrong type, returning default value
> >> (+ stack trace omitted)
> >>
> >> The warning is somewhat deceptive as the binding does refer to a boolean
> >> so it is not strictly of the wrong type, but an intermediate value
> >> encountered was null and this cannot be cast to a primitive boolean. It
> >> should probably just convert to the default value without any warning.
> >>
> >> So, to answer your question, I think we can indeed replace
> >> Bindings.select with the PoC's typesafe equivalent. There is almost no
> >> need to create specific primitive versions of this mechanism as users
> >> can use "orElse" to map to a suitable primitive value as the last step
> >> if null is undesired.
> >>
> >>> Do you think that this is a valid approach?
> >>
> >> Yes, I think our messages may have crossed paths as I suggested limiting
> >> the API (based on your earlier recommendations) in a post on the
> >> "Enhancements for JavaFX 18" thread in a reply to Kevin Rushforth.
> >>
> >> Your suggestion takes this a bit further by leaving the new methods in
> >> ObjectBinding package protected and reducing the new API in
> >> ObservableValue to its bare essentials. I think that's a fine approach
> >> as it keeps the API that we're commiting to small and allows the highest
> >> flexibility for future extensions.
> >>
> >> If an agreement can be reached on the initial API, I can rework the PoC
> >> and also add the unit tests (I'll need to convert the JUnit 5 tests I
> >> have to JUnit 4).
> >>
> >> --John
> >>
> >>>
> >>> - Nir
> >>>
> >>> On Wed, Apr 7, 2021 at 11:30 PM John Hendrikx <hj...@xs4all.nl
> >>> <mailto:hj...@xs4all.nl>> wrote:
> >>>
> >>> On 07/04/2021 03:41, Nir Lisker wrote:
> >>> > 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).
> >>>
> >>> Yes, it is the same concept as ReactFX calling a property "empty",
> >>> but I
> >>> was hesitant to call this as `null` is a valid value for many
> JavaFX
> >>> properties (a Scene can be null, a String can be null, etc.) which
> I
> >>> don't think means the same as it being empty (in the Optional
> sense).
> >>> But as long as the documentation is clear, I don't mind calling it
> >>> either.
> >>>
> >>> >
> >>> > 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.
> >>>
> >>> Yes, I think it is good to limit new API as much as possible to
> >> reduce
> >>> scope and increase the chances of its acceptance. The subscription
> >>> parts
> >>> can be designed separately and do not need to be public at this
> >> point.
> >>> They can be moved to a helper, or the implementation can take the
> >> extra
> >>> effort to use standard listeners.
> >>>
> >>> >
> >>> > 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.
> >>>
> >>> Yes, I think that would be good to do.
> >>>
> >>> I've done some comparisons myself and didn't find a difference in
> >>> functionality with `Val` (so far). It is a new implementation
> >> though, I
> >>> didn't really look at how `Val` was done internally as implementing
> >> it
> >>> directly into JavaFX is quite different (I had to make a few minor
> >>> changes in `ObjectBinding` to allow for the choice of lazy
> >> binding). I
> >>> was also initially more focused on Streams only to realize at a
> later
> >>> point that having a Stream implement ObservableValue was not going
> >>> to be
> >>> pretty (I suspect this also happened when ReactFX was created,
> which
> >> is
> >>> why Val/Var were later introduced in 2.x).
> >>>
> >>> Both the PoC and Val do lazy binding and are null safe and provide
> >>> methods to deal with null/empty.
> >>>
> >>> The main thing I didn't do yet is provide a `filter` method.
> >> Filtering
> >>> properties that you want to use for bindings seems awkard as a
> >> binding
> >>> should always have some kind of value. The `filter` method in
> ReactFX
> >>> basically maps the value to `null` when it doesn't match the
> filter.
> >>> I've left this out as you can easily achieve this with `map` and
> >>> `filter` seems to be too easy to misunderstand.
> >>>
> >>> Aside from that, ReactFX's Val offers a lot of other methods that
> >> are I
> >>> think a bit too specialized to consider at this point, like the
> >>> `animate`, `pin`, `mapDynamic` and `suspendable` methods.
> >>>
> >>> Val also has all the other `Optional` methods (ifPresent,
> isPresent,
> >>> isEmpty) but I think they may make the API a bit confusing (an
> >>> observable value is not the same as an optional). I've also not
> had a
> >>> need for these so far in practice and you can easily convert the
> >>> current
> >>> value to get this functionality with `Optional.ofNullable`.
> >>>
> >>> Finally Val offers a few methods to convert to ReactFX's streams.
> >> While
> >>> convenient, I think static methods like `Values.of`,
> >> `Invalidations.of`
> >>> or `Changes.of` would make for a less cluttered API to do stream
> >>> conversions -- this would also make it possible to leave this part
> of
> >>> the API up to a 3rd party.
> >>>
> >>> > By the way, do you make a distinction between ReactFX's Val and
> >>> Var in
> >>> > your proposal (one being read-only)?
> >>>
> >>> No, `ObservableValue` is basically the same as `Val`, and the
> >>> equivalent
> >>> to `Var` is `ObjectProperty`. Aside from it being a good companion
> >> to
> >>> `Val` (and less typing), I don't see a reason to implement `Var`.
> >>>
> >>> --John
> >>>
> >>> >
> >>> > On Sun, Apr 4, 2021 at 12:43 PM John Hendrikx <hj...@xs4all.nl
> >>> <mailto:hj...@xs4all.nl>
> >>> > <mailto:hj...@xs4all.nl <mailto: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 <mailto:hj...@xs4all.nl>
> >>> > <mailto:hj...@xs4all.nl <mailto: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
> >>> > >>
> >>> > >
> >>> >
> >>>
> >>
> >
>
