On Fri, 9 Jul 2021 22:17:21 GMT, Kevin Rushforth <k...@openjdk.org> wrote:
>> Michael Strauß has updated the pull request incrementally with one >> additional commit since the last revision: >> >> changes as per review > > modules/javafx.base/src/main/java/javafx/beans/property/Property.java line > 111: > >> 109: * established. However, doing so is not a meaningful use of this >> API, because the >> 110: * unidirectionally bound property will cause any attempt to change >> the value of any of >> 111: * the properties to fail with an exception. > > I don't think this quite matches the implementation. What I've observed is: > > * If a unidirectional binding already exists for this property, calling this > method to attempt to establish a bidirectional binding will immediately cause > an exception to happen. > * If a bidirectional binding already exists (as a result of calling this > method), subsequently calling the bind() method to set up a unidirectional > binding seems to "partially" remove the bidirectional binding. Setting the > other bidirectionally bound property no longer updates this property, but > setting this (or the newly bound property) does set the other bidirectionally > bound property. I very much doubt we want to specify this, though. > > More to the point, what we want to convey is that a property should not be > both bound unidirectionally and bidirectionally. So I'd like to see a > stronger "don't do that" sort of statement. The docs should state that an > application should not call this method if a unidirectional binding already > exists for either `this` property or the `other` property (rather than saying > that it's possible, but discouraged or not meaningful). I think it would be > fine to say that it is likely to cause an exception. > > Similarly, the docs for `bind` should say something about not calling that > method if `this` property is bidirectionally bound to any other property. Maybe we could simply the mental model of the property specification by making it illegal in all cases to use unidirectional and bidirectional bindings at the same time. The specification would be reduced to "it's illegal", instead of having a long explanation that says it's only _likely_ to cause an exception. Most attempts to use both kinds of bindings will fail with an exception anyway. Some will fail instantly when calling `bindBidirectional`, some will fail later when calling `setValue`. Some will fail more silently because the exception will happen inside `BidirectionalBinding` and not bring down the application. Only a kind-of-exotic situation will not produce exceptions at all (the situation that you described in the second bullet point). If we were to always fail instantly at the point of calling `bind` or `bindBidirectional`, we would make the misuse of this API visible where it's most relevant, instead of failing later in other parts of the codebase. ------------- PR: https://git.openjdk.java.net/jfx/pull/533
