RFR: 8268642: Expand the javafx.beans.property.Property documentation

[Nir Lisker]

On Mon, 14 Jun 2021 17:06:34 GMT, Michael Strauß <mstrauss at openjdk.org> wrote:

> * Expand the `Property.bind` and `Property.bindBidirectional` documentation
> * Change the name of the formal parameter of `Property.bind` to "source" (currently, it is inconsistently named "observable", "rawObservable" or "newObservable")

Looks good in general. I left some inline comments which are mostly optional.

modules/javafx.base/src/main/java/javafx/beans/property/BooleanPropertyBase.java line 165:

> 163:         if (source == null) {
> 164:             throw new NullPointerException("Cannot bind to null");
> 165:         }

I recommend using `Objects.requireNonNull(source, "Cannot bind to null");` while these are being changed. Same for other files in this PR.

modules/javafx.base/src/main/java/javafx/beans/property/BooleanPropertyBase.java line 167:

> 165:         }
> 166: 
> 167:         final ObservableBooleanValue newObservable = (source instanceof ObservableBooleanValue) ? (ObservableBooleanValue) source

This line is longer than 120 characters. You can break it into

        final ObservableBooleanValue newObservable = (source instanceof ObservableBooleanValue) ?
                (ObservableBooleanValue) source : new ValueWrapper(source);

modules/javafx.base/src/main/java/javafx/beans/property/Property.java line 47:

> 45:      * <p>
> 46:      * After establishing the binding, the value of the bound property is synchronized with the value
> 47:      * of the binding source. If the value of the binding source changes, the change is immediately

I would align the phrasing here with the one from bidirectional binding:

After establishing the binding, the value of the bound property is synchronized with the value
of the binding source: any change to the value of the binding source will immediately result in the value
of the bound property being changed accordingly.

modules/javafx.base/src/main/java/javafx/beans/property/Property.java line 68:

> 66: 
> 67:     /**
> 68:      * Removes a unidirectional binding that was established with {@link #bind(ObservableValue)}.

Why "**a** unidirectional binding" and not "**the**"? "a" implies that there can be more than one binding at the same time. At least that's how I read it.

modules/javafx.base/src/main/java/javafx/beans/property/Property.java line 115:

> 113:      * @param other the other property
> 114:      * @throws NullPointerException if {@code other} is {@code null}
> 115:      * @throws IllegalArgumentException if {@code other} is {@code this}

Since there are many references to unidirectional bindings, it might be wise to add an `@see` for the `bind` method. It could also be done with an `@link` on the first occurrence of "unidirectional binding".

modules/javafx.base/src/main/java/javafx/beans/property/Property.java line 120:

> 118: 
> 119:     /**
> 120:      * Removes a bidirectional binding that was established with {@link #bindBidirectional(Property)}.

"the" (it's possible to have multiple bidi bindings, but only 1 to the same `other` property).

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

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