Val.filer contains null if predicate is false

[jens-auer]

Hi,

I am using a Val bound to the text property of a TextField. I filter out strings which cannot be converted to LocalDateTime with a predicate in the string formatter

Val.filter(this.textField.textProperty(), this.format::isValidDateTime)

and them map the Val to LocalDateTime. In my program I start observing null values in the the Val when the isValidDateTime returns false. From the documentation of Val.filter, I would expect that values that do not satisfy the predicate are dropped and not replaced with null. However, I am not sure because I did not find a definition for an empty Val. Here is the code with the javadoc from

/**
 * Returns a new {@linkplain Val} that holds the same value
 * as this {@linkplain Val} when the value satisfies the predicate
 * and is empty when this {@linkplain Val} is empty or its value
 * does not satisfy the given predicate.
 */
default Val<T> filter(Predicate<? super T> p) {
    return filter(this, p);
}

static <T> Val<T> filter(
        ObservableValue<T> src,
        Predicate<? super T> p) {
    return map(src, t -> p.test(t) ? t : null);
}

It is obvious that values failing the predicate are substituted by null. Is that how it is supposed to work?

[jens-auer]

As a workaround, I do not use filter but replace values that do not satisfy the predicate with the previous value. This is then not forwarded as a change by the Var.

[TomasMikula]

Hi, yes, holding null is the intended meaning of "empty".

Val's getValue method must be able to return something at all times. So it returns null when nothing else is available.

Returning the previous value doesn't work when there is no previous value. In your case, that would be when the initial value of the testProperty() is not a valid date-time.

If you are not interested in calling getValue on the resulting Val anyway, then I suggest you use an EventStream instead:

EventStreams.valuesOf(this.textField.textProperty()).filterMap(s => toLocalDateTime(s))

where toLocalDateTime takes a String and returns Optional<LocalDateTime>.

[jens-auer]

Hi, this is similar to what I came up with. I am using an EventStreams.nonNullValues created from the Val's values() function. Your solution is much easier, so I think I am going to change my code.

[hjohn]

I must say the reactfx's behavior with nulls can be quite confusing, especially when null can be a valid value for a property (ie, a property that says something about the selected item, but is null when there currently is no selection).

For example, consider two properties A and B, either of which can be null. When I do:

Val.combine(A, B, (a, b) -> computeValue());

The combine function will not be called at all if either A or B contains null, even if an invalidation occured on the other property. Same for:

Val.create(() -> computeValue(), A, B);

The only one that works as desired is:

Val.create(() -> computeValue(), EventStreams.merge(A.changes(), B.changes());

Trying to change A and B to not use null but say an Optional quickly runs into issues because the standard mapping functions (filter, map, flatMap) can't be used anymore... for example, when using filter, it will revert to null again (instead of an empty Optional). And it's wierd anyway as Val already sort of is an Optional.

The only other option was something ugly like:

Val.combine(A.orElseConst(0), B.orElseConst(false), (a, b) -> computeValue());

Atleast computeValue is called then in all situations.

I'm happy enough with the EventStreams.merge solution, but it took quite a while to find, while dealing with all kinds of subtle bugs because one of my values contained null (I had 6 of them as input for the combine).