Re: Proof of concept for fluent bindings for ObservableValue
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:
Re: Proof of concept for fluent bindings for ObservableValue
I made the title more specific :)
--John
On 05/10/2021 17:58, Nir Lisker wrote:
Looks good. I assume we will add more bindings in the future like
conditionOn or anything else that I would put under "extras", so maybe
the title could be more specific since there will be more fluent
bindings in the future?
On Tue, Oct 5, 2021 at 1:34 PM John Hendrikx mailto:hj...@xs4all.nl>> wrote:
I've created https://bugs.openjdk.java.net/browse/JDK-8274771
Please have a look.
--John
On 04/10/2021 17:51, Nir Lisker wrote:
> I think that a PR can be created. The only point we need to decide
is about
> the subscription models we talked about above. ReactFX uses something
> different for each, you use the same. That can determine if we need
> different classes for each binding type.
>
> I can create the JBS issue for this one and a draft CSR if you want.
>
> On Tue, Sep 21, 2021 at 1:36 PM Nir Lisker mailto:nlis...@gmail.com>> wrote:
>
>> The OrElseBinding is incorrect
>>>
>>
>> Yes, that was a typo with the order of the arguments in the ternary
>> statement.
>>
>> I can rewrite the tests for JUnit 4 but I'd need a Nested runner (I
>>> think the tests would become pretty unreadable and less useful /
>>> thourough otherwise).
>>>
>>> What are the options?
>>>
>>
>> This is a bigger question. Kevin will have to answer that.
>>
>> As for the subscription model: flat map has a more complicated
one, but
>>> orElse, orElseGet and map all have the same one.
>>>
>>
>> From what I saw, ReactFX uses a different subscription model for
these. I
>> could have misread it.
>>
>> The messages will need to be written from the perspective of the
Binding
>>> class then IMHO.
>>>
>>
>> That's fine.
>>
>> As for the Optional methods, I'll have a look in my code base and
see if
>> the places I would like to use them at will become irrelevant
anyway with
>> the fluent bindings. I'm fine with proceeding with the current
names of the
>> proposed methods.
>>
>> On Sun, Sep 19, 2021 at 6:12 PM John Hendrikx mailto:hj...@xs4all.nl>> wrote:
>>
>>> I need to get you the tests I wrote, unfortunately, they're JUnit 5,
>>> please see the tests here:
>>>
>>>
>>>
https://github.com/hjohn/MediaSystem-v2/tree/master/mediasystem-jfx/src/test/java/javafx/beans/value
>>>
>>> The OrElseBinding is incorrect, the compute value should read:
>>>
>>> protected T computeValue() {
>>>T value = source.getValue();
>>>return value == null ? constant : value;
>>> }
>>>
>>> Similar for OrElseGetBinding.
>>>
>>> I can rewrite the tests for JUnit 4 but I'd need a Nested runner (I
>>> think the tests would become pretty unreadable and less useful /
>>> thourough otherwise).
>>>
>>> What are the options?
>>>
>>> - Integrate a nested runner (there is an Apache 2.0 licensed one
>>> available)
>>> - Create our own nested runner (about 200 lines of code)
>>> - Can we introduce JUnit 5?
>>> - Rewrite to plain JUnit 4?
>>>
>>> Let me know, and I can integrate them.
>>>
>>> --John
>>>
>>> On 19/09/2021 02:12, Nir Lisker wrote:
I've played around with the implementation and pushed a modified
prototype to the sandbox [1]. I ended up with something similar to
>>> ReactFX:
the orElse and orElseGet methods have their own binding classes
that
extend LazyObjectBinding, just like MapBinding and
FlatMapBinding. The
reason being that both their compute value and their
subscription models
are slightly different. While they can be matched to MapBinding
like you
did, it entails a bit of a roundabout way in my opinion. Creating a
supplier for the constant value of orElse somewhat defeats the
purpose I
think.
>>>
>>> I have no strong opinion here, just wanted to keep the MR small. The
>>> supplier should be an inline candidate, but creating a separate
class is
>>> fine too.
>>>
>>> As for the subscription model: flat map has a more complicated
one, but
>>> orElse, orElseGet and map all have the same one.
>>>
In addition, I think that it's fine to move the arguments' null
checks
>>> to
the binding classes themselves, even if it's a couple of levels
deeper
>>> on
the stack, while adding a message in the 2nd argument of
Objects.requireNonNull for clarity. These classes are
"self-contained"
>>> so
it makes sense for them to check their arguments. They might be
used in
other places, perhaps in the public Bindings class.
>>>
>>> The messages will need to be written from the perspective of the
B
Re: Proof of concept for fluent bindings for ObservableValue
Looks good. I assume we will add more bindings in the future like
conditionOn or anything else that I would put under "extras", so maybe the
title could be more specific since there will be more fluent bindings in
the future?
On Tue, Oct 5, 2021 at 1:34 PM John Hendrikx wrote:
> I've created https://bugs.openjdk.java.net/browse/JDK-8274771
>
> Please have a look.
>
> --John
>
> On 04/10/2021 17:51, Nir Lisker wrote:
> > I think that a PR can be created. The only point we need to decide is
> about
> > the subscription models we talked about above. ReactFX uses something
> > different for each, you use the same. That can determine if we need
> > different classes for each binding type.
> >
> > I can create the JBS issue for this one and a draft CSR if you want.
> >
> > On Tue, Sep 21, 2021 at 1:36 PM Nir Lisker wrote:
> >
> >> The OrElseBinding is incorrect
> >>>
> >>
> >> Yes, that was a typo with the order of the arguments in the ternary
> >> statement.
> >>
> >> I can rewrite the tests for JUnit 4 but I'd need a Nested runner (I
> >>> think the tests would become pretty unreadable and less useful /
> >>> thourough otherwise).
> >>>
> >>> What are the options?
> >>>
> >>
> >> This is a bigger question. Kevin will have to answer that.
> >>
> >> As for the subscription model: flat map has a more complicated one, but
> >>> orElse, orElseGet and map all have the same one.
> >>>
> >>
> >> From what I saw, ReactFX uses a different subscription model for these.
> I
> >> could have misread it.
> >>
> >> The messages will need to be written from the perspective of the Binding
> >>> class then IMHO.
> >>>
> >>
> >> That's fine.
> >>
> >> As for the Optional methods, I'll have a look in my code base and see if
> >> the places I would like to use them at will become irrelevant anyway
> with
> >> the fluent bindings. I'm fine with proceeding with the current names of
> the
> >> proposed methods.
> >>
> >> On Sun, Sep 19, 2021 at 6:12 PM John Hendrikx wrote:
> >>
> >>> I need to get you the tests I wrote, unfortunately, they're JUnit 5,
> >>> please see the tests here:
> >>>
> >>>
> >>>
> https://github.com/hjohn/MediaSystem-v2/tree/master/mediasystem-jfx/src/test/java/javafx/beans/value
> >>>
> >>> The OrElseBinding is incorrect, the compute value should read:
> >>>
> >>> protected T computeValue() {
> >>>T value = source.getValue();
> >>>return value == null ? constant : value;
> >>> }
> >>>
> >>> Similar for OrElseGetBinding.
> >>>
> >>> I can rewrite the tests for JUnit 4 but I'd need a Nested runner (I
> >>> think the tests would become pretty unreadable and less useful /
> >>> thourough otherwise).
> >>>
> >>> What are the options?
> >>>
> >>> - Integrate a nested runner (there is an Apache 2.0 licensed one
> >>> available)
> >>> - Create our own nested runner (about 200 lines of code)
> >>> - Can we introduce JUnit 5?
> >>> - Rewrite to plain JUnit 4?
> >>>
> >>> Let me know, and I can integrate them.
> >>>
> >>> --John
> >>>
> >>> On 19/09/2021 02:12, Nir Lisker wrote:
> I've played around with the implementation and pushed a modified
> prototype to the sandbox [1]. I ended up with something similar to
> >>> ReactFX:
> the orElse and orElseGet methods have their own binding classes that
> extend LazyObjectBinding, just like MapBinding and FlatMapBinding. The
> reason being that both their compute value and their subscription
> models
> are slightly different. While they can be matched to MapBinding like
> you
> did, it entails a bit of a roundabout way in my opinion. Creating a
> supplier for the constant value of orElse somewhat defeats the
> purpose I
> think.
> >>>
> >>> I have no strong opinion here, just wanted to keep the MR small. The
> >>> supplier should be an inline candidate, but creating a separate class
> is
> >>> fine too.
> >>>
> >>> As for the subscription model: flat map has a more complicated one, but
> >>> orElse, orElseGet and map all have the same one.
> >>>
> In addition, I think that it's fine to move the arguments' null checks
> >>> to
> the binding classes themselves, even if it's a couple of levels deeper
> >>> on
> the stack, while adding a message in the 2nd argument of
> Objects.requireNonNull for clarity. These classes are "self-contained"
> >>> so
> it makes sense for them to check their arguments. They might be used
> in
> other places, perhaps in the public Bindings class.
> >>>
> >>> The messages will need to be written from the perspective of the
> Binding
> >>> class then IMHO.
> >>>
> I also moved the subscription-related methods from ObservableValue to
> static methods in Subscription, at least for now, to defer the API
> >>> related
> to Subscription.
> >>>
> >>> Sounds good.
> >>>
> The branch doesn't compile because I didn't finish working on the
> visibility issue, but it's close enough to what I envision it like for
> >>> the
> first PR
Re: Proof of concept for fluent bindings for ObservableValue
I've created https://bugs.openjdk.java.net/browse/JDK-8274771
Please have a look.
--John
On 04/10/2021 17:51, Nir Lisker wrote:
I think that a PR can be created. The only point we need to decide is about
the subscription models we talked about above. ReactFX uses something
different for each, you use the same. That can determine if we need
different classes for each binding type.
I can create the JBS issue for this one and a draft CSR if you want.
On Tue, Sep 21, 2021 at 1:36 PM Nir Lisker wrote:
The OrElseBinding is incorrect
Yes, that was a typo with the order of the arguments in the ternary
statement.
I can rewrite the tests for JUnit 4 but I'd need a Nested runner (I
think the tests would become pretty unreadable and less useful /
thourough otherwise).
What are the options?
This is a bigger question. Kevin will have to answer that.
As for the subscription model: flat map has a more complicated one, but
orElse, orElseGet and map all have the same one.
From what I saw, ReactFX uses a different subscription model for these. I
could have misread it.
The messages will need to be written from the perspective of the Binding
class then IMHO.
That's fine.
As for the Optional methods, I'll have a look in my code base and see if
the places I would like to use them at will become irrelevant anyway with
the fluent bindings. I'm fine with proceeding with the current names of the
proposed methods.
On Sun, Sep 19, 2021 at 6:12 PM John Hendrikx wrote:
I need to get you the tests I wrote, unfortunately, they're JUnit 5,
please see the tests here:
https://github.com/hjohn/MediaSystem-v2/tree/master/mediasystem-jfx/src/test/java/javafx/beans/value
The OrElseBinding is incorrect, the compute value should read:
protected T computeValue() {
T value = source.getValue();
return value == null ? constant : value;
}
Similar for OrElseGetBinding.
I can rewrite the tests for JUnit 4 but I'd need a Nested runner (I
think the tests would become pretty unreadable and less useful /
thourough otherwise).
What are the options?
- Integrate a nested runner (there is an Apache 2.0 licensed one
available)
- Create our own nested runner (about 200 lines of code)
- Can we introduce JUnit 5?
- Rewrite to plain JUnit 4?
Let me know, and I can integrate them.
--John
On 19/09/2021 02:12, Nir Lisker wrote:
I've played around with the implementation and pushed a modified
prototype to the sandbox [1]. I ended up with something similar to
ReactFX:
the orElse and orElseGet methods have their own binding classes that
extend LazyObjectBinding, just like MapBinding and FlatMapBinding. The
reason being that both their compute value and their subscription models
are slightly different. While they can be matched to MapBinding like you
did, it entails a bit of a roundabout way in my opinion. Creating a
supplier for the constant value of orElse somewhat defeats the purpose I
think.
I have no strong opinion here, just wanted to keep the MR small. The
supplier should be an inline candidate, but creating a separate class is
fine too.
As for the subscription model: flat map has a more complicated one, but
orElse, orElseGet and map all have the same one.
In addition, I think that it's fine to move the arguments' null checks
to
the binding classes themselves, even if it's a couple of levels deeper
on
the stack, while adding a message in the 2nd argument of
Objects.requireNonNull for clarity. These classes are "self-contained"
so
it makes sense for them to check their arguments. They might be used in
other places, perhaps in the public Bindings class.
The messages will need to be written from the perspective of the Binding
class then IMHO.
I also moved the subscription-related methods from ObservableValue to
static methods in Subscription, at least for now, to defer the API
related
to Subscription.
Sounds good.
The branch doesn't compile because I didn't finish working on the
visibility issue, but it's close enough to what I envision it like for
the
first PR.
I've ported the changes over to my branch and ran the tests on it, they
all pass apart from the above mentioned problem in the OrElse bindings.
As for the java,util.Optional methods, I indeed did something like
`x.orElse(5).getValue()` in the past in other cases, but this approach
creates a new binding just to get the wrapped value out, which is very
inefficient. (In one case I did booleanValue.isNull().get(), which
creates
a boolean binding, because isPresent() does not exist). I would like to
see
what others think about the Optional methods, The binding method
variants
are much more important, but I want to avoid a name clash if the
Optional
ones will have support.
Okay, some more things to consider:
ObservableValue is not an Optional, their get methods respond
differently with Optional#get throwing an exception when null -- the
#orElse is a necessity; this is not the case for ObservableValue#getValue
Re: Proof of concept for fluent bindings for ObservableValue
On 04/10/2021 20:37, Michael Strauß wrote:
A little bit of bikeshedding here: I think this feature should be
thought of as a type-safe version of `Bindings.select`, which should
also be reflected in the terminology used (i.e. not map/flatMap). The
terminology, combined with the frequent comparisons to reactive
libraries might make it seem like an attempt to bring reactive
programming to JavaFX. That will certainly not happen, as JavaFX is
not (and never will be) a reactive framework. Naming it `select` will
convey the purpose and its place within JavaFX in a better and clearer
way.
The PoC is much more than that though. It allows arbitrary conversion of
property values and creating bindings for these that are automatically
kept updated. It not only removes the need for Bindings#select but also
sort of obsoletes many of the functions offered by
Object/String/IntegerExpression as for most of these #map can be used.
The terms #map and #flatMap (flatten + map) come from functional
programming which predates reactive programming, and do not necessarily
imply a stream as used by reactive. Java 8's Optional is a good example.
Both Optional and Stream are part of Java 8 and I contend are far better
known within the Java community than reactive programming. Therefore the
terms map and flatMap should be quite well understood by now.
--John
On Wed, Mar 24, 2021 at 10:49 PM 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(
Re: Proof of concept for fluent bindings for ObservableValue
On 04/10/2021 17:51, Nir Lisker wrote:
I think that a PR can be created. The only point we need to decide is
about the subscription models we talked about above. ReactFX uses
something different for each, you use the same. That can determine if we
need different classes for each binding type.
We can introduce classes for these in any case if that keeps things more
clean; they're not public (after your change) so we can change our minds
later.
I'm not sure I see the difference though, ReactFX does the same kind of
observations to keep the bindings up to date. Here's a comparison:
OrElseConst (ReactFX) vs OrElseBinding
--
protected Subscription connect() {
return Val.observeInvalidations(src, obs -> invalidate());
}
--
protected Subscription observeInputs() {
return Subscription.subscribeInvalidations(source,
this::invalidate); // start observing source
}
OrElseGetBinding (no equivalent in ReactFX)
---
@Override
protected Subscription observeInputs() {
return Subscription.subscribeInvalidations(source,
this::invalidate); // start observing source
}
FlatMapped (ReactFX) vs FlatMapBinding
--
protected final Subscription connect() {
return Val.observeInvalidations(src, obs -> srcInvalidated())
.and(this::stopObservingSelected);
}
private void stopObservingSelected() {
if(selectedSubscription != null) {
selectedSubscription.unsubscribe();
selectedSubscription = null;
}
}
--
protected Subscription observeInputs() {
Subscription subscription =
source.subscribeInvalidations(this::invalidate);
return () -> {
subscription.unsubscribe();
mappedSubscription.unsubscribe();
mappedSubscription = Subscription.EMPTY;
};
}
In both ReactFX and the PoC we see that FlatMapping requires a more
complicated subscription. As a FlatMap observes the original observable
and the result of the flat mapping, when unsubscribing two listeners
need to be unsubscribed.
As for the OrElse and OrElseGet, I just noticed ReactFX does not have an
#orElseGet equivalent. Closest thing is #getOrSupply but that doesn't
create a binding and is just a convenience method to get the current
value.
This does make me wonder how useful #orElseGet really would be -- I
think we should leave it out for now, it is easy to introduce later if
there is a real use case for it (I checked what I code I have, and I
never used it so far, it was only used to delegate #orElse to #orElseGet
which is pretty useless).
I think one more thing to decide is the naming of flatMap, there seems
to be some support for using "select" as a name instead. I personally
don't mind the name as for me flatMap just implies a map with the
additional step of flattening the result (stripping the extra wrapper,
be it Optional, Stream or in our case, ObservableValue).
I don't think that confusion with reactive programming is good argument
to not use this name, as neither Optional nor Stream have resulted in
such confusion.
I can create the JBS issue for this one and a draft CSR if you want.
It would be great to move this forward. If we can't borrow one of the
existing JBS issues then it would be good to create a new one, I can do
that, I can rewrite the text of the PoC MR a bit for this purpose. As
for a CSR, I'm not quite sure what that entails so it might be best if
you draft one.
The draft PR is mostly missing the JUnit test coverage now, which I can
take care off. I can also take care of the other changes we discussed
that may not be there yet. And I'll take another critical look at the
documentation and see if it still matches with what was discussed.
I've tried pushing the JUnit 5 upgrade as well, but I don't think it
will arrive in time (or perhaps it will). What I can do is convert my
JUnit 5 tests to JUnit 4, and once JUnit 5 is available I can revert that.
--John
Re: Proof of concept for fluent bindings for ObservableValue
A little bit of bikeshedding here: I think this feature should be
thought of as a type-safe version of `Bindings.select`, which should
also be reflected in the terminology used (i.e. not map/flatMap). The
terminology, combined with the frequent comparisons to reactive
libraries might make it seem like an attempt to bring reactive
programming to JavaFX. That will certainly not happen, as JavaFX is
not (and never will be) a reactive framework. Naming it `select` will
convey the purpose and its place within JavaFX in a better and clearer
way.
On Wed, Mar 24, 2021 at 10:49 PM 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 updateLongLivedModelWhileAvo
Re: Proof of concept for fluent bindings for ObservableValue
I think that a PR can be created. The only point we need to decide is about
the subscription models we talked about above. ReactFX uses something
different for each, you use the same. That can determine if we need
different classes for each binding type.
I can create the JBS issue for this one and a draft CSR if you want.
On Tue, Sep 21, 2021 at 1:36 PM Nir Lisker wrote:
> The OrElseBinding is incorrect
>>
>
> Yes, that was a typo with the order of the arguments in the ternary
> statement.
>
> I can rewrite the tests for JUnit 4 but I'd need a Nested runner (I
>> think the tests would become pretty unreadable and less useful /
>> thourough otherwise).
>>
>> What are the options?
>>
>
> This is a bigger question. Kevin will have to answer that.
>
> As for the subscription model: flat map has a more complicated one, but
>> orElse, orElseGet and map all have the same one.
>>
>
> From what I saw, ReactFX uses a different subscription model for these. I
> could have misread it.
>
> The messages will need to be written from the perspective of the Binding
>> class then IMHO.
>>
>
> That's fine.
>
> As for the Optional methods, I'll have a look in my code base and see if
> the places I would like to use them at will become irrelevant anyway with
> the fluent bindings. I'm fine with proceeding with the current names of the
> proposed methods.
>
> On Sun, Sep 19, 2021 at 6:12 PM John Hendrikx wrote:
>
>> I need to get you the tests I wrote, unfortunately, they're JUnit 5,
>> please see the tests here:
>>
>>
>> https://github.com/hjohn/MediaSystem-v2/tree/master/mediasystem-jfx/src/test/java/javafx/beans/value
>>
>> The OrElseBinding is incorrect, the compute value should read:
>>
>> protected T computeValue() {
>>T value = source.getValue();
>>return value == null ? constant : value;
>> }
>>
>> Similar for OrElseGetBinding.
>>
>> I can rewrite the tests for JUnit 4 but I'd need a Nested runner (I
>> think the tests would become pretty unreadable and less useful /
>> thourough otherwise).
>>
>> What are the options?
>>
>> - Integrate a nested runner (there is an Apache 2.0 licensed one
>> available)
>> - Create our own nested runner (about 200 lines of code)
>> - Can we introduce JUnit 5?
>> - Rewrite to plain JUnit 4?
>>
>> Let me know, and I can integrate them.
>>
>> --John
>>
>> On 19/09/2021 02:12, Nir Lisker wrote:
>> > I've played around with the implementation and pushed a modified
>> > prototype to the sandbox [1]. I ended up with something similar to
>> ReactFX:
>> > the orElse and orElseGet methods have their own binding classes that
>> > extend LazyObjectBinding, just like MapBinding and FlatMapBinding. The
>> > reason being that both their compute value and their subscription models
>> > are slightly different. While they can be matched to MapBinding like you
>> > did, it entails a bit of a roundabout way in my opinion. Creating a
>> > supplier for the constant value of orElse somewhat defeats the purpose I
>> > think.
>>
>> I have no strong opinion here, just wanted to keep the MR small. The
>> supplier should be an inline candidate, but creating a separate class is
>> fine too.
>>
>> As for the subscription model: flat map has a more complicated one, but
>> orElse, orElseGet and map all have the same one.
>>
>> > In addition, I think that it's fine to move the arguments' null checks
>> to
>> > the binding classes themselves, even if it's a couple of levels deeper
>> on
>> > the stack, while adding a message in the 2nd argument of
>> > Objects.requireNonNull for clarity. These classes are "self-contained"
>> so
>> > it makes sense for them to check their arguments. They might be used in
>> > other places, perhaps in the public Bindings class.
>>
>> The messages will need to be written from the perspective of the Binding
>> class then IMHO.
>>
>> > I also moved the subscription-related methods from ObservableValue to
>> > static methods in Subscription, at least for now, to defer the API
>> related
>> > to Subscription.
>>
>> Sounds good.
>>
>> > The branch doesn't compile because I didn't finish working on the
>> > visibility issue, but it's close enough to what I envision it like for
>> the
>> > first PR.
>>
>> I've ported the changes over to my branch and ran the tests on it, they
>> all pass apart from the above mentioned problem in the OrElse bindings.
>>
>> > As for the java,util.Optional methods, I indeed did something like
>> > `x.orElse(5).getValue()` in the past in other cases, but this approach
>> > creates a new binding just to get the wrapped value out, which is very
>> > inefficient. (In one case I did booleanValue.isNull().get(), which
>> creates
>> > a boolean binding, because isPresent() does not exist). I would like to
>> see
>> > what others think about the Optional methods, The binding method
>> variants
>> > are much more important, but I want to avoid a name clash if the
>> Optional
>> > ones will have support.
>>
>> Okay, some more things to consider:
Re: Re: Proof of concept for fluent bindings for ObservableValue
I’m no expert here so take it with a grain of salt but…
I was proposing moving this sorts of ObservableValue type interfaces and
implementations out of jfx (and React) context and into JDK context. Then
these sorts of observable type properties/ bindings can be used outside of
just UI contexts.
Eric
On Tue, Sep 21, 2021 at 12:27 PM Nir Lisker wrote:
> I'm not sure what you're proposing. Move what, to where, and for what use?
>
> On Tue, Sep 21, 2021 at 8:02 PM Eric Bresie wrote:
>
>> I’m very much an Observer here ;-) but
>>
>> Given the comparisons with FX and React implementations, is there value
>> in moving some of this out of here and into the JDK proper context making
>> it potentially usable outside of fx or react circles?
>>
>> I’m reminded of old JDK workings when someone might be working a headless
>> application needing a Swing dependency.
>>
>> Eric Bresie
>> ebre...@gmail.com (mailto:ebre...@gmail.com)
>
>
>>
>> > On September 21, 2021 at 5:36:13 AM CDT, Nir Lisker > (mailto:nlis...@gmail.com)> wrote:
>> > >
>> > > The OrElseBinding is incorrect
>> > >
>> >
>> > Yes, that was a typo with the order of the arguments in the ternary
>> > statement.
>> >
>> > I can rewrite the tests for JUnit 4 but I'd need a Nested runner (I
>> > > think the tests would become pretty unreadable and less useful /
>> > > thourough otherwise).
>> > >
>> > > What are the options?
>> > >
>> >
>> > This is a bigger question. Kevin will have to answer that.
>> >
>> > As for the subscription model: flat map has a more complicated one, but
>> > > orElse, orElseGet and map all have the same one.
>> > >
>> >
>> > From what I saw, ReactFX uses a different subscription model for these.
>> I
>> > could have misread it.
>> >
>> > The messages will need to be written from the perspective of the Binding
>> > > class then IMHO.
>> > >
>> >
>> > That's fine.
>> >
>> > As for the Optional methods, I'll have a look in my code base and see if
>> > the places I would like to use them at will become irrelevant anyway
>> with
>> > the fluent bindings. I'm fine with proceeding with the current names of
>> the
>> > proposed methods.
>> >
>> > On Sun, Sep 19, 2021 at 6:12 PM John Hendrikx > hj...@xs4all.nl)> wrote:
>> >
>> > > I need to get you the tests I wrote, unfortunately, they're JUnit 5,
>> > > please see the tests here:
>> > >
>> > >
>> > >
>> https://github.com/hjohn/MediaSystem-v2/tree/master/mediasystem-jfx/src/test/java/javafx/beans/value
>> > >
>> > > The OrElseBinding is incorrect, the compute value should read:
>> > >
>> > > protected T computeValue() {
>> > > T value = source.getValue();
>> > > return value == null ? constant : value;
>> > > }
>> > >
>> > > Similar for OrElseGetBinding.
>> > >
>> > > I can rewrite the tests for JUnit 4 but I'd need a Nested runner (I
>> > > think the tests would become pretty unreadable and less useful /
>> > > thourough otherwise).
>> > >
>> > > What are the options?
>> > >
>> > > - Integrate a nested runner (there is an Apache 2.0 licensed one
>> available)
>> > > - Create our own nested runner (about 200 lines of code)
>> > > - Can we introduce JUnit 5?
>> > > - Rewrite to plain JUnit 4?
>> > >
>> > > Let me know, and I can integrate them.
>> > >
>> > > --John
>> > >
>> > > On 19/09/2021 02:12, Nir Lisker wrote:
>> > > > I've played around with the implementation and pushed a modified
>> > > > prototype to the sandbox [1]. I ended up with something similar to
>> > > ReactFX:
>> > > > the orElse and orElseGet methods have their own binding classes that
>> > > > extend LazyObjectBinding, just like MapBinding and FlatMapBinding.
>> The
>> > > > reason being that both their compute value and their subscription
>> models
>> > > > are slightly different. While they can be matched to MapBinding
>> like you
>> > > > did, it entails a bit of a roundabout way in my opinion. Creating a
>> > > > supplier for the constant value of orElse somewhat defeats the
>> purpose I
>> > > > think.
>> > >
>> > > I have no strong opinion here, just wanted to keep the MR small. The
>> > > supplier should be an inline candidate, but creating a separate class
>> is
>> > > fine too.
>> > >
>> > > As for the subscription model: flat map has a more complicated one,
>> but
>> > > orElse, orElseGet and map all have the same one.
>> > >
>> > > > In addition, I think that it's fine to move the arguments' null
>> checks to
>> > > > the binding classes themselves, even if it's a couple of levels
>> deeper on
>> > > > the stack, while adding a message in the 2nd argument of
>> > > > Objects.requireNonNull for clarity. These classes are
>> "self-contained" so
>> > > > it makes sense for them to check their arguments. They might be
>> used in
>> > > > other places, perhaps in the public Bindings class.
>> > >
>> > > The messages will need to be written from the perspective of the
>> Binding
>> > > class then IMHO.
>> > >
>> > > > I also moved the subscription-related methods from ObservableValue
>> to
>> > > > static meth
Re: Re: Proof of concept for fluent bindings for ObservableValue
I'm not sure what you're proposing. Move what, to where, and for what use?
On Tue, Sep 21, 2021 at 8:02 PM Eric Bresie wrote:
> I’m very much an Observer here ;-) but
>
> Given the comparisons with FX and React implementations, is there value in
> moving some of this out of here and into the JDK proper context making it
> potentially usable outside of fx or react circles?
>
> I’m reminded of old JDK workings when someone might be working a headless
> application needing a Swing dependency.
>
> Eric Bresie
> ebre...@gmail.com (mailto:ebre...@gmail.com)
>
> > On September 21, 2021 at 5:36:13 AM CDT, Nir Lisker (mailto:nlis...@gmail.com)> wrote:
> > >
> > > The OrElseBinding is incorrect
> > >
> >
> > Yes, that was a typo with the order of the arguments in the ternary
> > statement.
> >
> > I can rewrite the tests for JUnit 4 but I'd need a Nested runner (I
> > > think the tests would become pretty unreadable and less useful /
> > > thourough otherwise).
> > >
> > > What are the options?
> > >
> >
> > This is a bigger question. Kevin will have to answer that.
> >
> > As for the subscription model: flat map has a more complicated one, but
> > > orElse, orElseGet and map all have the same one.
> > >
> >
> > From what I saw, ReactFX uses a different subscription model for these. I
> > could have misread it.
> >
> > The messages will need to be written from the perspective of the Binding
> > > class then IMHO.
> > >
> >
> > That's fine.
> >
> > As for the Optional methods, I'll have a look in my code base and see if
> > the places I would like to use them at will become irrelevant anyway with
> > the fluent bindings. I'm fine with proceeding with the current names of
> the
> > proposed methods.
> >
> > On Sun, Sep 19, 2021 at 6:12 PM John Hendrikx hj...@xs4all.nl)> wrote:
> >
> > > I need to get you the tests I wrote, unfortunately, they're JUnit 5,
> > > please see the tests here:
> > >
> > >
> > >
> https://github.com/hjohn/MediaSystem-v2/tree/master/mediasystem-jfx/src/test/java/javafx/beans/value
> > >
> > > The OrElseBinding is incorrect, the compute value should read:
> > >
> > > protected T computeValue() {
> > > T value = source.getValue();
> > > return value == null ? constant : value;
> > > }
> > >
> > > Similar for OrElseGetBinding.
> > >
> > > I can rewrite the tests for JUnit 4 but I'd need a Nested runner (I
> > > think the tests would become pretty unreadable and less useful /
> > > thourough otherwise).
> > >
> > > What are the options?
> > >
> > > - Integrate a nested runner (there is an Apache 2.0 licensed one
> available)
> > > - Create our own nested runner (about 200 lines of code)
> > > - Can we introduce JUnit 5?
> > > - Rewrite to plain JUnit 4?
> > >
> > > Let me know, and I can integrate them.
> > >
> > > --John
> > >
> > > On 19/09/2021 02:12, Nir Lisker wrote:
> > > > I've played around with the implementation and pushed a modified
> > > > prototype to the sandbox [1]. I ended up with something similar to
> > > ReactFX:
> > > > the orElse and orElseGet methods have their own binding classes that
> > > > extend LazyObjectBinding, just like MapBinding and FlatMapBinding.
> The
> > > > reason being that both their compute value and their subscription
> models
> > > > are slightly different. While they can be matched to MapBinding like
> you
> > > > did, it entails a bit of a roundabout way in my opinion. Creating a
> > > > supplier for the constant value of orElse somewhat defeats the
> purpose I
> > > > think.
> > >
> > > I have no strong opinion here, just wanted to keep the MR small. The
> > > supplier should be an inline candidate, but creating a separate class
> is
> > > fine too.
> > >
> > > As for the subscription model: flat map has a more complicated one, but
> > > orElse, orElseGet and map all have the same one.
> > >
> > > > In addition, I think that it's fine to move the arguments' null
> checks to
> > > > the binding classes themselves, even if it's a couple of levels
> deeper on
> > > > the stack, while adding a message in the 2nd argument of
> > > > Objects.requireNonNull for clarity. These classes are
> "self-contained" so
> > > > it makes sense for them to check their arguments. They might be used
> in
> > > > other places, perhaps in the public Bindings class.
> > >
> > > The messages will need to be written from the perspective of the
> Binding
> > > class then IMHO.
> > >
> > > > I also moved the subscription-related methods from ObservableValue to
> > > > static methods in Subscription, at least for now, to defer the API
> > > related
> > > > to Subscription.
> > >
> > > Sounds good.
> > >
> > > > The branch doesn't compile because I didn't finish working on the
> > > > visibility issue, but it's close enough to what I envision it like
> for
> > > the
> > > > first PR.
> > >
> > > I've ported the changes over to my branch and ran the tests on it, they
> > > all pass apart from the above mentioned problem in the OrElse bindings.
> > >
> > > > As for the java,util.
Re: Re: Proof of concept for fluent bindings for ObservableValue
I’m very much an Observer here ;-) but
Given the comparisons with FX and React implementations, is there value in
moving some of this out of here and into the JDK proper context making it
potentially usable outside of fx or react circles?
I’m reminded of old JDK workings when someone might be working a headless
application needing a Swing dependency.
Eric Bresie
ebre...@gmail.com (mailto:ebre...@gmail.com)
> On September 21, 2021 at 5:36:13 AM CDT, Nir Lisker (mailto:nlis...@gmail.com)> wrote:
> >
> > The OrElseBinding is incorrect
> >
>
> Yes, that was a typo with the order of the arguments in the ternary
> statement.
>
> I can rewrite the tests for JUnit 4 but I'd need a Nested runner (I
> > think the tests would become pretty unreadable and less useful /
> > thourough otherwise).
> >
> > What are the options?
> >
>
> This is a bigger question. Kevin will have to answer that.
>
> As for the subscription model: flat map has a more complicated one, but
> > orElse, orElseGet and map all have the same one.
> >
>
> From what I saw, ReactFX uses a different subscription model for these. I
> could have misread it.
>
> The messages will need to be written from the perspective of the Binding
> > class then IMHO.
> >
>
> That's fine.
>
> As for the Optional methods, I'll have a look in my code base and see if
> the places I would like to use them at will become irrelevant anyway with
> the fluent bindings. I'm fine with proceeding with the current names of the
> proposed methods.
>
> On Sun, Sep 19, 2021 at 6:12 PM John Hendrikx (mailto:hj...@xs4all.nl)> wrote:
>
> > I need to get you the tests I wrote, unfortunately, they're JUnit 5,
> > please see the tests here:
> >
> >
> > https://github.com/hjohn/MediaSystem-v2/tree/master/mediasystem-jfx/src/test/java/javafx/beans/value
> >
> > The OrElseBinding is incorrect, the compute value should read:
> >
> > protected T computeValue() {
> > T value = source.getValue();
> > return value == null ? constant : value;
> > }
> >
> > Similar for OrElseGetBinding.
> >
> > I can rewrite the tests for JUnit 4 but I'd need a Nested runner (I
> > think the tests would become pretty unreadable and less useful /
> > thourough otherwise).
> >
> > What are the options?
> >
> > - Integrate a nested runner (there is an Apache 2.0 licensed one available)
> > - Create our own nested runner (about 200 lines of code)
> > - Can we introduce JUnit 5?
> > - Rewrite to plain JUnit 4?
> >
> > Let me know, and I can integrate them.
> >
> > --John
> >
> > On 19/09/2021 02:12, Nir Lisker wrote:
> > > I've played around with the implementation and pushed a modified
> > > prototype to the sandbox [1]. I ended up with something similar to
> > ReactFX:
> > > the orElse and orElseGet methods have their own binding classes that
> > > extend LazyObjectBinding, just like MapBinding and FlatMapBinding. The
> > > reason being that both their compute value and their subscription models
> > > are slightly different. While they can be matched to MapBinding like you
> > > did, it entails a bit of a roundabout way in my opinion. Creating a
> > > supplier for the constant value of orElse somewhat defeats the purpose I
> > > think.
> >
> > I have no strong opinion here, just wanted to keep the MR small. The
> > supplier should be an inline candidate, but creating a separate class is
> > fine too.
> >
> > As for the subscription model: flat map has a more complicated one, but
> > orElse, orElseGet and map all have the same one.
> >
> > > In addition, I think that it's fine to move the arguments' null checks to
> > > the binding classes themselves, even if it's a couple of levels deeper on
> > > the stack, while adding a message in the 2nd argument of
> > > Objects.requireNonNull for clarity. These classes are "self-contained" so
> > > it makes sense for them to check their arguments. They might be used in
> > > other places, perhaps in the public Bindings class.
> >
> > The messages will need to be written from the perspective of the Binding
> > class then IMHO.
> >
> > > I also moved the subscription-related methods from ObservableValue to
> > > static methods in Subscription, at least for now, to defer the API
> > related
> > > to Subscription.
> >
> > Sounds good.
> >
> > > The branch doesn't compile because I didn't finish working on the
> > > visibility issue, but it's close enough to what I envision it like for
> > the
> > > first PR.
> >
> > I've ported the changes over to my branch and ran the tests on it, they
> > all pass apart from the above mentioned problem in the OrElse bindings.
> >
> > > As for the java,util.Optional methods, I indeed did something like
> > > `x.orElse(5).getValue()` in the past in other cases, but this approach
> > > creates a new binding just to get the wrapped value out, which is very
> > > inefficient. (In one case I did booleanValue.isNull().get(), which
> > creates
> > > a boolean binding, because isPresent() does not exist). I would like to
> > see
> > > what others t
Re: Proof of concept for fluent bindings for ObservableValue
>
> The OrElseBinding is incorrect
>
Yes, that was a typo with the order of the arguments in the ternary
statement.
I can rewrite the tests for JUnit 4 but I'd need a Nested runner (I
> think the tests would become pretty unreadable and less useful /
> thourough otherwise).
>
> What are the options?
>
This is a bigger question. Kevin will have to answer that.
As for the subscription model: flat map has a more complicated one, but
> orElse, orElseGet and map all have the same one.
>
>From what I saw, ReactFX uses a different subscription model for these. I
could have misread it.
The messages will need to be written from the perspective of the Binding
> class then IMHO.
>
That's fine.
As for the Optional methods, I'll have a look in my code base and see if
the places I would like to use them at will become irrelevant anyway with
the fluent bindings. I'm fine with proceeding with the current names of the
proposed methods.
On Sun, Sep 19, 2021 at 6:12 PM John Hendrikx wrote:
> I need to get you the tests I wrote, unfortunately, they're JUnit 5,
> please see the tests here:
>
>
> https://github.com/hjohn/MediaSystem-v2/tree/master/mediasystem-jfx/src/test/java/javafx/beans/value
>
> The OrElseBinding is incorrect, the compute value should read:
>
> protected T computeValue() {
>T value = source.getValue();
>return value == null ? constant : value;
> }
>
> Similar for OrElseGetBinding.
>
> I can rewrite the tests for JUnit 4 but I'd need a Nested runner (I
> think the tests would become pretty unreadable and less useful /
> thourough otherwise).
>
> What are the options?
>
> - Integrate a nested runner (there is an Apache 2.0 licensed one available)
> - Create our own nested runner (about 200 lines of code)
> - Can we introduce JUnit 5?
> - Rewrite to plain JUnit 4?
>
> Let me know, and I can integrate them.
>
> --John
>
> On 19/09/2021 02:12, Nir Lisker wrote:
> > I've played around with the implementation and pushed a modified
> > prototype to the sandbox [1]. I ended up with something similar to
> ReactFX:
> > the orElse and orElseGet methods have their own binding classes that
> > extend LazyObjectBinding, just like MapBinding and FlatMapBinding. The
> > reason being that both their compute value and their subscription models
> > are slightly different. While they can be matched to MapBinding like you
> > did, it entails a bit of a roundabout way in my opinion. Creating a
> > supplier for the constant value of orElse somewhat defeats the purpose I
> > think.
>
> I have no strong opinion here, just wanted to keep the MR small. The
> supplier should be an inline candidate, but creating a separate class is
> fine too.
>
> As for the subscription model: flat map has a more complicated one, but
> orElse, orElseGet and map all have the same one.
>
> > In addition, I think that it's fine to move the arguments' null checks to
> > the binding classes themselves, even if it's a couple of levels deeper on
> > the stack, while adding a message in the 2nd argument of
> > Objects.requireNonNull for clarity. These classes are "self-contained" so
> > it makes sense for them to check their arguments. They might be used in
> > other places, perhaps in the public Bindings class.
>
> The messages will need to be written from the perspective of the Binding
> class then IMHO.
>
> > I also moved the subscription-related methods from ObservableValue to
> > static methods in Subscription, at least for now, to defer the API
> related
> > to Subscription.
>
> Sounds good.
>
> > The branch doesn't compile because I didn't finish working on the
> > visibility issue, but it's close enough to what I envision it like for
> the
> > first PR.
>
> I've ported the changes over to my branch and ran the tests on it, they
> all pass apart from the above mentioned problem in the OrElse bindings.
>
> > As for the java,util.Optional methods, I indeed did something like
> > `x.orElse(5).getValue()` in the past in other cases, but this approach
> > creates a new binding just to get the wrapped value out, which is very
> > inefficient. (In one case I did booleanValue.isNull().get(), which
> creates
> > a boolean binding, because isPresent() does not exist). I would like to
> see
> > what others think about the Optional methods, The binding method variants
> > are much more important, but I want to avoid a name clash if the Optional
> > ones will have support.
>
> Okay, some more things to consider:
>
> ObservableValue is not an Optional, their get methods respond
> differently with Optional#get throwing an exception when null -- the
> #orElse is a necessity; this is not the case for ObservableValue#getValue.
>
> When creating mapping chains, you are going to do this because you want
> to bind this to another property or to listen on it (with subscribe).
> You don't want to do this as a one time thing. If you are creating a
> chain just to calculate a value you can just do this directly. Instead of:
>
> widthPro
Re: Proof of concept for fluent bindings for ObservableValue
I need to get you the tests I wrote, unfortunately, they're JUnit 5,
please see the tests here:
https://github.com/hjohn/MediaSystem-v2/tree/master/mediasystem-jfx/src/test/java/javafx/beans/value
The OrElseBinding is incorrect, the compute value should read:
protected T computeValue() {
T value = source.getValue();
return value == null ? constant : value;
}
Similar for OrElseGetBinding.
I can rewrite the tests for JUnit 4 but I'd need a Nested runner (I
think the tests would become pretty unreadable and less useful /
thourough otherwise).
What are the options?
- Integrate a nested runner (there is an Apache 2.0 licensed one available)
- Create our own nested runner (about 200 lines of code)
- Can we introduce JUnit 5?
- Rewrite to plain JUnit 4?
Let me know, and I can integrate them.
--John
On 19/09/2021 02:12, Nir Lisker wrote:
I've played around with the implementation and pushed a modified
prototype to the sandbox [1]. I ended up with something similar to ReactFX:
the orElse and orElseGet methods have their own binding classes that
extend LazyObjectBinding, just like MapBinding and FlatMapBinding. The
reason being that both their compute value and their subscription models
are slightly different. While they can be matched to MapBinding like you
did, it entails a bit of a roundabout way in my opinion. Creating a
supplier for the constant value of orElse somewhat defeats the purpose I
think.
I have no strong opinion here, just wanted to keep the MR small. The
supplier should be an inline candidate, but creating a separate class is
fine too.
As for the subscription model: flat map has a more complicated one, but
orElse, orElseGet and map all have the same one.
In addition, I think that it's fine to move the arguments' null checks to
the binding classes themselves, even if it's a couple of levels deeper on
the stack, while adding a message in the 2nd argument of
Objects.requireNonNull for clarity. These classes are "self-contained" so
it makes sense for them to check their arguments. They might be used in
other places, perhaps in the public Bindings class.
The messages will need to be written from the perspective of the Binding
class then IMHO.
I also moved the subscription-related methods from ObservableValue to
static methods in Subscription, at least for now, to defer the API related
to Subscription.
Sounds good.
The branch doesn't compile because I didn't finish working on the
visibility issue, but it's close enough to what I envision it like for the
first PR.
I've ported the changes over to my branch and ran the tests on it, they
all pass apart from the above mentioned problem in the OrElse bindings.
As for the java,util.Optional methods, I indeed did something like
`x.orElse(5).getValue()` in the past in other cases, but this approach
creates a new binding just to get the wrapped value out, which is very
inefficient. (In one case I did booleanValue.isNull().get(), which creates
a boolean binding, because isPresent() does not exist). I would like to see
what others think about the Optional methods, The binding method variants
are much more important, but I want to avoid a name clash if the Optional
ones will have support.
Okay, some more things to consider:
ObservableValue is not an Optional, their get methods respond
differently with Optional#get throwing an exception when null -- the
#orElse is a necessity; this is not the case for ObservableValue#getValue.
When creating mapping chains, you are going to do this because you want
to bind this to another property or to listen on it (with subscribe).
You don't want to do this as a one time thing. If you are creating a
chain just to calculate a value you can just do this directly. Instead of:
widthProperty().map(x -> x * 2).getValue()
You'd do:
getWidth() * 2;
With flat mapping however this is not as easy:
[1]
node.sceneProperty()
.flatMap(Scene::windowProperty)
.flatMap(Window::showingProperty)
.orElse(false);
You could try:
node.getScene().getWindow().isShowing();
But that will not work when Scene or Window is null. You'd have to
write it as:
[2]
Optional.ofNullable(node.getScene())
.map(Scene::getWindow)
.map(Window::isShowing)
.orElse(false);
If you only offer a "specialized" orElse, you'd still need to create
several bindings:
[3]
node.sceneProperty()
.flatMap(Scene::windowProperty)
.flatMap(Window::showingProperty)
.orElse2(false); // orElse which returns a value not a binding
If you compare [2] (which is possible now) with [3] the difference is
minimal. A bit more ceremony at the start for [2] but a shorter, cleaner
and faster mapping (no bindings and no need to use the property methods).
Now, if you already HAVE the binding for some other purpose, then it is
highly likely it also already has an #orElse that is needed as part of
the binding. In that case callin
Re: Proof of concept for fluent bindings for ObservableValue
I've played around with the implementation and pushed a modified prototype to the sandbox [1]. I ended up with something similar to ReactFX: the orElse and orElseGet methods have their own binding classes that extend LazyObjectBinding, just like MapBinding and FlatMapBinding. The reason being that both their compute value and their subscription models are slightly different. While they can be matched to MapBinding like you did, it entails a bit of a roundabout way in my opinion. Creating a supplier for the constant value of orElse somewhat defeats the purpose I think. In addition, I think that it's fine to move the arguments' null checks to the binding classes themselves, even if it's a couple of levels deeper on the stack, while adding a message in the 2nd argument of Objects.requireNonNull for clarity. These classes are "self-contained" so it makes sense for them to check their arguments. They might be used in other places, perhaps in the public Bindings class. I also moved the subscription-related methods from ObservableValue to static methods in Subscription, at least for now, to defer the API related to Subscription. The branch doesn't compile because I didn't finish working on the visibility issue, but it's close enough to what I envision it like for the first PR. As for the java,util.Optional methods, I indeed did something like `x.orElse(5).getValue()` in the past in other cases, but this approach creates a new binding just to get the wrapped value out, which is very inefficient. (In one case I did booleanValue.isNull().get(), which creates a boolean binding, because isPresent() does not exist). I would like to see what others think about the Optional methods, The binding method variants are much more important, but I want to avoid a name clash if the Optional ones will have support. [1] https://github.com/openjdk/jfx-sandbox/tree/jfx-sandbox/nlisker_fuent_bindings On Wed, Sep 15, 2021 at 1:59 PM John Hendrikx wrote: > > > On 15/09/2021 02:28, Nir Lisker wrote: > > 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. 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 defaultSupplier) > > Val orElseConst(T other) > > Val orElse(ObservableValue other) > > I see what you mean now. The methods from java.util.Optional will return > an unwrapped value, while the ones from ObservableValue in the PoC > return an ObservableValue, but they have the same name. > > So java.util.Optional offers: > > T orElse(T other) > T orElseGet(Supplier supplier) > > And the PoC: > > ObservableValue orElse(T alternativeValue) > ObservableValue orElseGet(Supplier supplier) > > The main difference is in the returned types. Personally, I think it is > rarely useful for a binding to be queried directly and I've never used > the #getOrElse or #getOrSupply variants that ReactFX offers. On top of > that: > > x.orElse(5).getValue()===x.getOrElse(5) > > So it introduces another method in the interface to avoid having to > write ".getValue()". The opposite, introducing only the "unwrapped" > variants would still require the "wrapped" variants to be present as well. > > So, what I would suggest is to not add variants for #getOrElse and > #getOrSupply at all as they are of questionable value and would just > bloat the API for a bit less typing. In that case I think we can still > use the signatures as they are. > > ReactFX also offers: > > Val orElse(ObservableValue other) > > This is another rarely used feature IMHO, and I think Optional#or would > a better match for this functionality: > > Optional or(Supplier> supplier) > > In the POC the signature would be: > > ObservableValue or( >Supplier> supplier > ) > > I didn't implement this one in the PoC to keep it small, but I did > implement this in my JavaFX EventStream library before. > > > 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. > > I think #ifPresent can be added, not so sure about the usefulness of > #getOrElse (see above). > > > 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
Re: Proof of concept for fluent bindings for ObservableValue
On 15/09/2021 02:28, Nir Lisker wrote: 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. 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 defaultSupplier) Val orElseConst(T other) Val orElse(ObservableValue other) I see what you mean now. The methods from java.util.Optional will return an unwrapped value, while the ones from ObservableValue in the PoC return an ObservableValue, but they have the same name. So java.util.Optional offers: T orElse(T other) T orElseGet(Supplier supplier) And the PoC: ObservableValue orElse(T alternativeValue) ObservableValue orElseGet(Supplier supplier) The main difference is in the returned types. Personally, I think it is rarely useful for a binding to be queried directly and I've never used the #getOrElse or #getOrSupply variants that ReactFX offers. On top of that: x.orElse(5).getValue()===x.getOrElse(5) So it introduces another method in the interface to avoid having to write ".getValue()". The opposite, introducing only the "unwrapped" variants would still require the "wrapped" variants to be present as well. So, what I would suggest is to not add variants for #getOrElse and #getOrSupply at all as they are of questionable value and would just bloat the API for a bit less typing. In that case I think we can still use the signatures as they are. ReactFX also offers: Val orElse(ObservableValue other) This is another rarely used feature IMHO, and I think Optional#or would a better match for this functionality: Optional or(Supplier> supplier) In the POC the signature would be: ObservableValue or( Supplier> supplier ) I didn't implement this one in the PoC to keep it small, but I did implement this in my JavaFX EventStream library before. 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. I think #ifPresent can be added, not so sure about the usefulness of #getOrElse (see above). 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: Sorry, I see it now. You have a good point, the current implementation indeed wraps another Lambda to add the null check which could have been done in #computeValue. I think it would be a good move to avoid the extra lambda simply because the end result would be more readable -- any performance improvement would be a bonus, I don't know if there will be any. 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. Perhaps this is useful: https://github.com/hjohn/MediaSystem-v2/tree/master/mediasystem-jfx When you add this as a maven dependency to your project, you will get the new PoC functionality. It basically uses the Maven shade plugin to replace a few classes in javafx-base -- I use this sometimes to fix bugs I need fixed immediately by patching jfx, but found it also works very nicely to experiment with this PoC. Also, the PoC PR compiles fine, it may need rebasing. 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? That should be no problem, I can split it up. 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. Yes, I think we can easily do without for now. 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).
Re: Proof of concept for fluent bindings for ObservableValue
>
> 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. 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 defaultSupplier)
Val orElseConst(T other)
Val orElse(ObservableValue 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 ObservableValue mapping(ObservableValue
source, Function mapper) {
return nullableMapping(source, mapper.apply(v)); // don't deal with a
null v here
}
public static ObservableValue
nullableMapping(ObservableValue source, Function
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 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 orE
Re: Proof of concept for fluent bindings for ObservableValue
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
Re: Proof of concept for fluent bindings for ObservableValue
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.
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?
3. Why does nullableMapping creates an anonymous subtype of LazyBinding,
while flatMapping creates a concrete instance of FlatMapBinding?
4. Why does orElseGet call Bindings.nullableMapping directly, while map
calls Bindings.mapping which in turn calls Bindings.nullableMapping?
5. I noticed also that subscribeInvalidations in ObservableValue will need
to be hidden.
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
https://bugs.openjdk.java.net/browse/JDK-8091316
On Mon, Sep 13, 2021 at 3:05 AM John Hendrikx 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")
>
> ve
Re: Proof of concept for fluent bindings for ObservableValue
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)
.
Re: Proof of concept for fluent bindings for ObservableValue
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. 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? 2. If we want to replace the Bindings.select (non-type safe) API, can we do it with our current way of treating null? Do you think that this is a valid approach? - Nir On Wed, Apr 7, 2021 at 11:30 PM John Hendrikx 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
Re: Proof of concept for fluent bindings for ObservableValue
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 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 conc
Re: Proof of concept for fluent bindings for ObservableValue
>
> 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 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 thes
Re: Proof of concept for fluent bindings for ObservableValue
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 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
Re: Proof of concept for fluent bindings for ObservableValue
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.
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.
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.
The third part is for collections - things like transformation lists
(LiveList) and for other collections.
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.
- 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 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:
>labe
Proof of concept for fluent bindings for ObservableValue
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... flu
