> 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

Reply via email to