[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-openjfx-dev
Subject:    Re: RFR: 8264770: BidirectionalBinding should use InvalidationListener to prevent boxing [v2]
From:       John Hendrikx <jhendrikx () openjdk ! java ! net>
Date:       2021-08-28 20:36:31
Message-ID: 2aE82u5hRBt2GNATrMrzvVlEZ-wponOr-rassUe7nEw=.18b74ef6-919c-4f1a-8a13-aace0bfc1722 () github ! com
[Download RAW message or body]

On Fri, 14 May 2021 22:30:16 GMT, Michael Strauß <mstrauss@openjdk.org> wrote:

> > The internal BidirectionalBinding class implements bidirectional bindings for \
> > JavaFX properties. The design intent of this class is to provide specializations \
> > for primitive value types to prevent boxing conversions (cf. specializations of \
> > the Property class with a similar design intent). 
> > However, the primitive BidirectionalBinding implementations do not meet the \
> > design goal of preventing boxing conversions, because they implement \
> > ChangeListener. 
> > ChangeListener is a generic SAM interface, which makes it impossibe to invoke an \
> > implementation of ChangeListener::changed with a primitive value (i.e. any \
> > primitive value will be auto-boxed). 
> > The boxing conversion happens, as with all ChangeListeners, at the invocation \
> > site (for example, in ExpressionHelper). Since the boxing conversion has already \
> > happened by the time any of the BidirectionalBinding implementations is invoked, \
> > there's no point in using primitive specializations of BidirectionalBinding after \
> > the fact. 
> > This issue can be solved by having BidirectionalBinding implement \
> > InvalidationListener instead, which by itself does not incur a boxing conversion. \
> > Because bidirectional bindings are eagerly evaluated, the observable behavior \
> > remains the same. 
> > I've filed a bug report with the same title.
> 
> Michael Strauß has updated the pull request incrementally with one additional \
> commit since the last revision: 
> added missing oldValue assignments

modules/javafx.base/src/main/java/com/sun/javafx/binding/BidirectionalBinding.java \
line 157:

> 155: 
> 156:         property1.setValue((T)property2.getValue());
> 157:         property1.addListener(binding);

About lines 156-158:

        property1.setValue(property2.getValue());
        property1.addListener(binding);
        property2.addListener(binding);

The bindings added do not validate the properties anymore as it is an invalidation \
listener now instead of a change listener.  This doesn't matter for `property2` as \
its `getValue` is called which will force its revalidation, but for `property1` this \
is not the case.  This small program demonstrates this:

    SimpleDoubleProperty p = new SimpleDoubleProperty(2);

    InvalidationListener invalidationListener = obs -> \
System.out.println("Invalidated");

    p.addListener(invalidationListener);

    p.setValue(3);
    p.setValue(4);

The program as expected only prints `invalidated` once.

-------------

PR: https://git.openjdk.java.net/jfx/pull/454


[prev in list] [next in list] [prev in thread] [next in thread]