RFR: 8290040: Provide simplified deterministic way to manage listeners [v2]

John Hendrikx jhendrikx at openjdk.org
Thu Sep 1 13:41:33 UTC 2022


On Wed, 31 Aug 2022 17:19:37 GMT, Nir Lisker <nlisker at openjdk.org> wrote:

>> John Hendrikx has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Rename showing property to shown as it is already used in subclasses
>
> 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());
        }
    }

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

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


More information about the openjfx-dev mailing list