RFR: 8268642: Improve property system to facilitate correct usage [v2]
Michael Strauß
mstrauss at openjdk.org
Wed Jan 4 03:52:39 UTC 2023
> 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<E> 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:~~
>
> <U> void bindBidirectional(Property<U> other, StringConverter<U> converter)
>
> ~~The inherited `Property.unbindBidirectional(Property<String>)` API cannot be used to unbind a conversion binding, because it only accepts arguments of type `Property<String>`.
> `StringProperty` solves this issue by adding an untyped overload: `unbindBidirectional(Object)`.~~
>
> ~~I propose to relax the definition of `Property.unbindBidirectional`:~~
>
> - void unbindBidirectional(Property<T> other);
> + void unbindBidirectional(Property<?> other);
>
> ~~This allows property implementations to use the same API to un-bind regular bidirectional bindings, as well as converting bidirectional bindings. The `Property` typing is retained with a minimal impact on existing code (most _usages_ of this API will continue to compile, while classes that override `unbindBidirectional` will need to be changed to match the new API).~~
>
> ~~As a side-effect, this allows the option of easily adding conversion bindings for other property implementations as well.~~
Michael Strauß has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 29 commits:
- use pattern variable
- removed unused return value
- update copyright headers
- Merge branch 'master' into feature/property-enhancements
- centralized Property.toString
- test error message in content binding tests
- fix whitespace issues
- fix things that broke
- Clean up content binding API
- Rework exception messages
- ... and 19 more: https://git.openjdk.org/jfx/compare/a35c3bf7...86280bea
-------------
Changes: https://git.openjdk.org/jfx/pull/590/files
Webrev: https://webrevs.openjdk.org/?repo=jfx&pr=590&range=01
Stats: 4740 lines in 94 files changed: 3084 ins; 879 del; 777 mod
Patch: https://git.openjdk.org/jfx/pull/590.diff
Fetch: git fetch https://git.openjdk.org/jfx pull/590/head:pull/590
PR: https://git.openjdk.org/jfx/pull/590
More information about the openjfx-dev
mailing list