Re: RFR: 8268642: Improve property system to facilitate correct usage
On Tue, 27 Jul 2021 23:15:10 GMT, Michael Strauß wrote: > 3. Align un-binding of unidirectional content bindings with regular > unidirectional bindings Same comment as for item 4. We need to finish the discussion of whether and how to do this. - PR: https://git.openjdk.java.net/jfx/pull/590
Re: RFR: 8268642: Improve property system to facilitate correct usage
On Tue, 27 Jul 2021 23:15:10 GMT, Michael Strauß wrote:
> Based on previous discussions, this PR attempts to improve the JavaFX
> property system by enforcing correct API usage in several cases that are
> outlined below. It also streamlines the API by deprecating untyped APIs in
> favor of typed APIs that better express intent.
>
> ### 1. Behavioral changes for regular bindings
>
> var target = new SimpleObjectProperty(bean, "target");
> var source = new SimpleObjectProperty(bean, "source");
> target.bind(source);
> target.bindBidirectional(source);
>
> _Before:_ `RuntimeException` --> "bean.target: A bound value cannot be set."
> _After:_ `IllegalStateException` --> "bean.target: Bidirectional binding
> cannot target a bound property."
>
>
> var target = new SimpleObjectProperty(bean, "target");
> var source = new SimpleObjectProperty(bean, "source");
> var other = new SimpleObjectProperty(bean, "other");
> source.bind(other);
> target.bindBidirectional(source);
>
> _Before:_ no error
> _After:_ `IllegalArgumentException` --> "bean.source: Bidirectional binding
> cannot target a bound property."
>
>
> var target = new SimpleObjectProperty(bean, "target");
> var source = new SimpleObjectProperty(bean, "source");
> target.bindBidirectional(source);
> target.bind(source);
>
> _Before:_ no error
> _After:_ `IllegalStateException` --> "bean.target: Cannot bind a property
> that is targeted by a bidirectional binding."
>
>
> ### 2. Behavioral changes for content bindings
>
> var target = new SimpleListProperty(bean, "target");
> var source = new SimpleListProperty(bean, "source");
> target.bindContent(source);
> target.bindContentBidirectional(source);
>
> _Before:_ no error
> _After:_ `IllegalStateException` --> "bean.target: Bidirectional content
> binding cannot target a bound collection."
>
>
> var target = new SimpleListProperty(bean, "target");
> var source = new SimpleListProperty(bean, "source");
> var other = new SimpleListProperty();
> source.bindContent(other);
> target.bindContentBidirectional(source);
>
> _Before:_ no error
> _After:_ `IllegalArgumentException` --> "bean.source: Bidirectional content
> binding cannot target a bound collection."
>
>
> var target = new SimpleListProperty(bean, "target");
> var source = new SimpleListProperty(bean, "source");
> target.bindContentBidirectional(source);
> target.bindContent(source);
>
> _Before:_ no error
> _After:_ `IllegalStateException` --> "bean.target: Cannot bind a collection
> that is targeted by a bidirectional content binding."
>
>
> var target = new SimpleListProperty(bean, "target");
> var source = new SimpleListProperty(bean, "source");
> target.bindContent(source);
> target.set(FXCollections.observableArrayList());
>
> _Before:_ no error
> _After:_ `IllegalStateException` --> "bean.target: Cannot set the value of a
> content-bound property."
>
>
> var target = new SimpleListProperty(bean, "target");
> var source = new SimpleListProperty(bean, "source");
> target.bindContent(source);
> target.add("foo");
>
> _Before_: no error, but binding is broken because the lists are in an
> inconsistent state
> _After:_ `RuntimeExeption` via `Thread.UncaughtExceptionHandler` --> "Illegal
> list modification: Content binding was removed because the lists are
> out-of-sync."
>
>
> ### 3. Align un-binding of unidirectional content bindings with regular
> unidirectional bindings
> The API of unidirectional content bindings is aligned with regular
> unidirectional bindings because the semantics are similar. Like `unbind()`,
> `unbindContent(Object)` should not need an argument because there can only be
> a single content binding. For this reason, `unbindContent(Object)` will be
> deprecated in favor of `unbindContent()`:
>
> void bindContent(ObservableList source);
> + void unbindContent();
> + boolean isContentBound();
>
> + @Deprecated(since = "18", forRemoval = true)
> void unbindContent(Object object);
>
>
> ### 4. Replace untyped binding APIs with typed APIs
> The following untyped APIs will be marked for removal in favor of typed
> replacements:
> In `javafx.beans.binding.Bindings`:
>
> @Deprecated(since = "18", forRemoval = true)
> void unbindBidirectional(Object property1, Object property2)
>
> @Deprecated(since = "18", forRemoval = true)
> void unbindContentBidirectional(Object obj1, Object obj2)
>
> @Deprecated(since = "18", forRemoval = true)
> void unbindContent(Object obj1, Object obj2)
>
>
> In `ReadOnlyListProperty`, `ReadOnlySetProperty`, `ReadOnlyMapProperty`:
>
> @Deprecated(since = "18", forRemoval = true)
> void unbindContentBidirectional(Object object)
>
>
> ~~5. Support un-binding bidirectional conversion bindings with typed API
> At this moment, `StringProperty` is the only property implementation that
> offers bidirectional conversion bindings, where the `StringProperty` is bound
> to a property of another type:~~
>
> void bindBidirectional(Property other,
Re: Improve property system to facilitate correct usage
I'd like to discuss the API changes surrounding content bindings for this PR: https://github.com/openjdk/jfx/pull/590 Content bindings in the property system are semantically similar to regular bindings: 1. You can only have a single content binding for a collection-type property 2. Unidirectional content bindings and bidirectional content bindings are mutually exclusive For this reason, and in order to support the behavioral changes outlined in the PR, the content binding API (in ReadOnlyListProperty) was changed as follows: void bindContent(ObservableList source); + @Deprecated(since = "18", forRemoval = true) void unbindContent(Object object); + void unbindContent(); + boolean isContentBound(); This is not a breaking change, and allows application developers to phase out the superfluous argument for `unbindContent` over time. What might be more controversial is that this imposes a new implementation requirement for some exotic implementations of collection-type properties: 1. Right now, content bindings are implemented in ReadOnlyListProperty, and every implementation of ROLP inherits the default content binding implementation. 2. With the changed API, the implementation moves from ReadOnlyListProperty to (ReadOnly)ListPropertyBase. This means that a class that implements (RO)LP, but not (RO)LPBase, does not inherit the default content binding implementation and needs to provide its own implementation if it wants to support content bindings. Of course, implementing (RO)LP without using (RO)LPBase is extremely unusual. It requires the implementer to re-implement lots of code like `ListExpressionHelper` and binding listeners. Still, it's an additional requirement for these implementations that also want to support content bindings. If content bindings are not required, then the "old" (RO)LP implementation will continue to compile and work. What we get from this change are powerful correctness guarantees for users of the API. Do you think this is a trade worth making? On Wed, Jul 28, 2021 at 1:23 AM Michael Strauß wrote: > > I propose a set of changes to the JavaFX property system that I've > outlined in this PR: https://github.com/openjdk/jfx/pull/590 > > The changes fall into two categories: enforcement of correct usage > (there are several cases listed in the PR), and deprecating untyped > APIs (for removal in a future version) so as to make the intent of the > API more clear to developers. > > Even though there are breaking changes, the impact on application code > should be minimal. I'd welcome any comments on this proposal.
[External] : Re: Improve property system to facilitate correct usage
> > Yes, that's the big problem. Binary compatibility is very important. The > value proposition of making the types a bit more clear doesn't justify > breaking binary compatibility. Binary compatibility certainly is a valid point, but I disagree. I think consistency is more important. Users and devs will have to cope with the changes. It is important, but we can't always think about binary compatibility, "breaking" changes are sometimes needed
Re: [External] : Re: Improve property system to facilitate correct usage
some changes are binary incompatible, they are syntactically transparent. Yes, that's the big problem. Binary compatibility is very important. The value proposition of making the types a bit more clear doesn't justify breaking binary compatibility. -- Kevin On 7/27/2021 6:09 PM, Michael Strauß wrote: I should point out that the rest of the JavaFX framework did not require a single code change as a result of the API changes. So while some changes are binary incompatible, they are syntactically transparent. Am Mi., 28. Juli 2021 um 01:39 Uhr schrieb Kevin Rushforth : This will take a while to work through, and we will need to get general consensus on the API changes. I doubt I can support incompatible breaking changes in this area, given how fundamental property and bindings are to JavaFX. I'll take a look, but it is likely that the incompatible API changes part of your proposed change will not be accepted. The changes enforcing correct usage should be a lot less controversial and easier to get through. -- Kevin On 7/27/2021 4:23 PM, Michael Strauß wrote: I propose a set of changes to the JavaFX property system that I've outlined in this PR: https://urldefense.com/v3/__https://github.com/openjdk/jfx/pull/590__;!!ACWV5N9M2RV99hQ!egor-rENORC_wn09Va7FeNBmsgCGsCm3yzccC3yvO_x-wuuyMXrzpE_OplPNe2pJWeue$ The changes fall into two categories: enforcement of correct usage (there are several cases listed in the PR), and deprecating untyped APIs (for removal in a future version) so as to make the intent of the API more clear to developers. Even though there are breaking changes, the impact on application code should be minimal. I'd welcome any comments on this proposal.
Re: Improve property system to facilitate correct usage
I should point out that the rest of the JavaFX framework did not require a single code change as a result of the API changes. So while some changes are binary incompatible, they are syntactically transparent. Am Mi., 28. Juli 2021 um 01:39 Uhr schrieb Kevin Rushforth : > > This will take a while to work through, and we will need to get general > consensus on the API changes. > > I doubt I can support incompatible breaking changes in this area, given > how fundamental property and bindings are to JavaFX. I'll take a look, > but it is likely that the incompatible API changes part of your proposed > change will not be accepted. > > The changes enforcing correct usage should be a lot less controversial > and easier to get through. > > -- Kevin > > > On 7/27/2021 4:23 PM, Michael Strauß wrote: > > I propose a set of changes to the JavaFX property system that I've > > outlined in this PR: https://github.com/openjdk/jfx/pull/590 > > > > The changes fall into two categories: enforcement of correct usage > > (there are several cases listed in the PR), and deprecating untyped > > APIs (for removal in a future version) so as to make the intent of the > > API more clear to developers. > > > > Even though there are breaking changes, the impact on application code > > should be minimal. I'd welcome any comments on this proposal. >
Re: Improve property system to facilitate correct usage
This will take a while to work through, and we will need to get general consensus on the API changes. I doubt I can support incompatible breaking changes in this area, given how fundamental property and bindings are to JavaFX. I'll take a look, but it is likely that the incompatible API changes part of your proposed change will not be accepted. The changes enforcing correct usage should be a lot less controversial and easier to get through. -- Kevin On 7/27/2021 4:23 PM, Michael Strauß wrote: I propose a set of changes to the JavaFX property system that I've outlined in this PR: https://github.com/openjdk/jfx/pull/590 The changes fall into two categories: enforcement of correct usage (there are several cases listed in the PR), and deprecating untyped APIs (for removal in a future version) so as to make the intent of the API more clear to developers. Even though there are breaking changes, the impact on application code should be minimal. I'd welcome any comments on this proposal.
Improve property system to facilitate correct usage
I propose a set of changes to the JavaFX property system that I've outlined in this PR: https://github.com/openjdk/jfx/pull/590 The changes fall into two categories: enforcement of correct usage (there are several cases listed in the PR), and deprecating untyped APIs (for removal in a future version) so as to make the intent of the API more clear to developers. Even though there are breaking changes, the impact on application code should be minimal. I'd welcome any comments on this proposal.
