ObservableValue.map special-cases the null value

[John Hendrikx]

On 18/05/2023 19:05, Michael Strauß wrote:
> Hi John,
>
> the `isPresent()` and `isEmtpy()` methods don't feel entirely natural
> to me. I think the problem is that Optional has slightly different
> semantics compared to ObservableValue. An Optional either has a value,
> or it doesn't. The "it doesn't have a value" state doesn't feel
> exactly equivalent to "having the null value", since no Optional API
> will ever expose the null value. In contrast, ObservableValue will
> happily expose the null value.

ObservableValue is more of an Optional that is mutable (but is presented 
read only). It's certainly not a perfect match, but most methods that 
make sense for Optional also make good sense for ObservableValue. The 
main difference is that some methods of Optional return a final result 
(which is fine because it is immutable), while the same method for 
ObservableValue must return another ObservableValue (as it can change at 
any time, being mutable):

       optional.orElse(x)  -->  fixed value
       optional.map(...) --> another Optional

Vs:

       observableValue.orElse(x)  -->  another ObservableValue
       observableValue.map(...)  -->  another ObservableValue

Optional's isPresent and isEmpty return booleans; in the observable 
world, where the value can change, it makes sense that the return values 
themselves would be observables.  Other methods we may "steal" that I've 
considered (but not found a good use case for yet) are:

      or  --  can be useful as it is hard/impossible to do with current 
primitives
      filter  --  "filter(x -> x == 2)" would be equivalent to "map(x -> 
x == 2 ? x : null)" but more convenient
      orElseGet  -- a dynamic variant of orElse: orElseGet(() -> 
getResourceBundle(locale).get("empty.text"))

Methods that make no sense for a mutable Optional are:

       ifPresent -- would have to be called "ifCurrentlyPresent"... 
makes no sense
       ifPresentOrElse  -- a combination of map/orElse (and a very ugly 
method with 2 lambda's)
       orElseThrow  -- seems pointless, this would be better as a 
getValue alternative (getValueOrThrowIfNull)
       stream -- would have to be a hot stream, which the Stream API 
doesn't do (cold streams only AFAIK)


> While the map(...).orElse(...) combination is sufficient for some
> scenarios, it doesn't allow to disambiguate the case where map()
> returns `null` from the case where map() was elided because the input
> argument was `null`. That's where mapNullable() would be materially
> different. But I acknowledge that this is an edge case.

Optional does this as well, you can't tell if it was already empty or if 
it has become empty due to mapping to null:

       assertEquals( Optional.empty(), Optional.ofNullable(x).map(x -> 
null) )

What would be the use case for wanting to know if the current value was 
null already or if your mapping function made it null (or perhaps the 
next mapping function did it?) It seems to me that if you want to know 
if it is currently null, you can just call `getValue`.  If you don't 
know what your mapping function is doing (ie, it may sometimes return 
null) that seems like a problem that need not be solved by ObservableValue.

--John


>
> On Wed, May 17, 2023 at 3:08 AM John Hendrikx <john.hendrikx at gmail.com> wrote:
>> Hi Michael,
>>
>> As you're only trying to map to true/false, it is a bit ugly to do this
>> I suppose:
>>
>>         nullableValue.map(x -> true).orElse(false);
>>
>> I've encountered situations like these before, where you only want to
>> know about the presence of a value, and don't care what the value is.
>> If there are sufficient use cases, we could consider adding a method for
>> this.  Instead of `mapNullable` though I was thinking more along the
>> lines of `isPresent` and `isEmpty`:
>>
>>         ObservableValue<Boolean> isPresent();  // true if not null
>>
>>         ObservableValue<Boolean> isEmpty();  // true if null
>>
>> So far I didn't think it would be warranted to add such methods, but
>> they're trivial to add.
>>
>> On 17/05/2023 00:25, Michael Strauß wrote:
>>> I'm trying to map an ObservableValue such that `null` is mapped to
>>> `false` and any other value is mapped to `true`. The following line of
>>> code should do the trick:
>>> ObservableValue<Boolean> mappedValue = nullableValue.map(x -> x != null);
>>>
>>> It turns out that this doesn't work, since `map` doesn't pass `null`
>>> to the mapping function. Instead of calling the mapping function, it
>>> always returns `null`. This is not technically a bug, as `map` is
>>> specified to have this behavior. It is a bit unfortunate, though;
>>> `null` is a perfectly valid value in many cases and its special
>>> treatment comes off as a bit of a surprise.
>> The alternative is worse I think, as JavaFX properties can easily be
>> `null`, you'd have to add null checks to your map/flatMaps. Especially a
>> construct like this:
>>
>>         node.sceneProperty()
>>             .flatMap(Scene::windowProperty)
>>             .flatMap(Window::showingProperty)
>>             .orElse(false);
>>
>> ... would be really annoying to write if you had to deal with the
>> nulls.  The functionality is closely related to what Optional offers,
>> and that one doesn't pass `null` to its map/flatMap methods either, nor
>> will it ever add a `mapNullable` :-)
>>
>>> I'm wondering whether we need a `mapNullable` operator that does what
>>> I intuitively expecting `map` to do, which is just unconditionally
>>> passing the value to the mapping function.
>> Maybe, I think however that you'd only need this for the very special
>> case where you are intending to ignore the actual value being passed to
>> a map/flatMap, and only want to check it for null.  `isPresent` /
>> `isEmpty` would be nicer for this.  For all other cases, where you are
>> actually doing some kind of mapping, I think a map/orElse combo works great.
>>
>> --John
>>