On Thu, 1 Sep 2022 13:38:46 GMT, John Hendrikx <jhendr...@openjdk.org> wrote:

>> modules/javafx.base/src/main/java/javafx/beans/value/ObservableValue.java 
>> line 274:
>> 
>>> 272:      * <p>
>>> 273:      * Returning {@code null} from the given condition is treated the 
>>> same as
>>> 274:      * returning {@code false}.
>> 
>> I would rephrase to use the "holds" language instead of the "return":
>> 
>>> `{@code condition}` holding `{@code null}` is treated the same as it 
>>> holding `{@code false}`
>> 
>> But is this the behavior we want? I would consider throwing when the 
>> condition is not a `true` or `false` because it's a boolean condition. 
>> `orElse` can be used to create a condition that maps `null` to something 
>> else if the user wants to accept `null`s in the condition. I'm not sure if 
>> this is really a problem.
>
> I've rephrased it as suggested.
> 
> As for `null` -- I've clarified here what it currently does (so it is not 
> left undefined), but I have no strong feelings either way. 
> 
> I mainly choose this direction because JavaFX has so far opted to treat 
> `null` as something to be ignored or converted to a default value (for 
> `BooleanProperty`, you can set it to `null` using the generic setter, and it 
> would become `false`).  Since the fluent system is basically built on top of 
> `ObjectProperty` (and thus `Object`) we can't easily exclude the possibility 
> of it being `null` (I have to accept `ObservableValue<Boolean>` instead of 
> `BooleanProperty`).
> 
> ie:
> 
>      listView.layoutXProperty().setValue(null);
> 
> makes no sense at all, but JavaFX accepts it and uses `0.0`.
> 
> Ouch, just noticed it always creates (not throws) an exception (with 
> expensive stacktrace) whenever you do that, despite it maybe not getting 
> logged:
> 
>     @Override
>     public void setValue(Number v) {
>         if (v == null) {
>             Logging.getLogger().fine("Attempt to set double property to null, 
> using default value instead.", new NullPointerException());
>             set(0.0);
>         } else {
>             set(v.doubleValue());
>         }
>     }

Yeah, the behavior of the layout property is not the best in my opinion. Let's 
see what others think.

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

PR: https://git.openjdk.org/jfx/pull/830

Reply via email to