On Fri, 12 Aug 2022 17:30:20 GMT, Andy Goryachev <[email protected]> wrote:
>> modules/javafx.controls/src/main/java/javafx/scene/control/Control.java line
>> 245:
>>
>>> 243: if (skin != null) {
>>> 244: if (skin.getSkinnable() != Control.this) {
>>> 245: throw new IllegalArgumentException("There must be
>>> 1:1 relationship between Skin and Skinnable");
>>
>> I wonder an error message like "Skin was not created for this Skinnable" or
>> "Skin does not correspond to this Skinnable" would be clearer?
>>
>> For the implementation, you should also unbind, if needed, before throwing
>> the exception (we really should have made this a read-only property so it
>> couldn't be bound, since a Skin is a "use once" object).
>>
>>
>> if (isBound()) {
>> unbind();
>> }
>
> Good point!
> Actually, unbind() is sufficient, as it does the required check. From
> javadoc: _If the Property is not bound, calling this method has no effect._
> Is there another reason to call isBound(), perhaps for clarity?
Good question. I do see the check in other places where an exception is thrown
from invalidated, such as TextField, TextArea, TableColumnHeader. More
importantly, those properties also restore their previous value (irrespective
of binding), which we might want to consider here.
-------------
PR: https://git.openjdk.org/jfx/pull/845
