On Tue, 4 Oct 2022 15:06:30 GMT, Kevin Rushforth <k...@openjdk.org> wrote:
>> modules/javafx.controls/src/main/java/javafx/scene/control/Control.java line
>> 229:
>>
>>> 227: * against this Control, and throw an {@code
>>> IllegalArgumentException} if it is not the same.
>>> 228: * <p>
>>> 229: * A skin may be null.
>>
>> This breaks the 1-1 relationship mentioned above, so it's probably best to
>> mention this as an exception to the 1-1 rule.
>
> Maybe something like `will check the return value of {@link
> Skin#getSkinnable()}, if the Skin is not {@code null}, against this
> Control...`?
How about
* To ensure a one-to-one relationship between a {@code Skinnable} and its
* {@code Skin}, some implementations of {@link Skinnable#setSkin(Skin)}
method will check
* the return value of {@link Skin#getSkinnable()}, and if it is not {@code
null}, compare it
* with this Skinnable, throwing an {@code IllegalArgumentException} if it
is not the same.
>> modules/javafx.controls/src/main/java/javafx/scene/control/Skinnable.java
>> line 43:
>>
>>> 41: * It listens and responds to changes in state in a {@code
>>> Skinnable}.
>>> 42: * <p>
>>> 43: * There is typically a one-to-one relationship between a {@code
>>> Skinnable} and its
>>
>> The word "typically" doesn't describe when this applies and when not. From
>> the previous comments, I get a better understanding (PopupControl), but
>> developers reading the JavaDoc won't see that.
>
> Good point. We have a tension between "pure OO" and giving clear advice. In
> the case of `Skin`, there are only two direct subclasses, so we could call
> that out here and say that `Control` enforces a 1-to-1 relationship, but
> `PopupControl` does not. Alternatively, we could change this to say that
> "Some implementations of Skinnable define a 1-to-1 relationship...". That
> would key developers to look for that in the the documentation of the
> implementing class.
>
> Which way do you think is best?
I like "Some implementations of Skinnable define a 1-to-1 relationship..."...
-------------
PR: https://git.openjdk.org/jfx/pull/845
