Refactor ValueWrapper into [value, valueOrNull, hasValue]

[frankpepermans]

...Just a test to see if we can simplify things without requiring a ValueWrapper,

this PR is not complete, but just an example of an idea:
instead of having a ValueWrapper:

T get value; // can throw if value is not yet emitted, and the Stream's Type is non-nullable
T? get valueOrNull; // same as above, but returns null and does not throw
bool get hasValue; // returns true if an event is played and value was set, _even if that event is 'null'!_

[larssn]

Thanks for working on a solution! 😊

I gotta say I'm not the biggest fan of the whole "storing the value in multiple fields". It can make things overly complicated. More than that, I don't understand why valueOrNull is needed in the first place, as T from value can be nullable (implicitly), as I attempted to demonstrate in our thread.

Here's a more verbose example - try it in DartPad

class SimpleBehaviorSubject<T> {
  T _val;
  T get value => _val;
  
  SimpleBehaviorSubject(this._val);
  
  @override
  String toString() {
    return _val == null ? 'T was null' : _val.toString();
  }
}

void main() {
  final unseeded = SimpleBehaviorSubject<num?>(null);
  print(unseeded);
  
  final seeded = SimpleBehaviorSubject(1);
  print(seeded);
}

As you can see, T can be nullable without the ?.
So why do we need the valueOrNull field again?

[frankpepermans]

For convenience really,

I noticed that for example list.firstWhereOrNull is very popular, also a lot of existing rxdart code probably has stream.value != null checks already.

So either they just use value, or switch over to valueOrNull for the same behavior.

HasValue is there, because you might want to safely check this first, before calling value.
And trickier, if the Stream is T?, then before adding events, it is false, but if you explicitly add null, it becomes true, but valueOrNull yields null in both cases.

[frankpepermans]

Your example only works when you pass a seed value, but the whole complexity lies with value being not nullable, but also not being seeded,

If you check BehaviorSubject sources for example, you can see how cumbersome it is to support it.

[hoc081098]

firstWhereOrNull

That is it. From my Kotlin background, Kotlin always two ways to get something: xxxOrNull and xxx (can throw error) (eg. Iterable#firstOrNull and Iterable#first - throws NoSuchElementException when iterable is empty).
This is convenience when using without try { } catch { } 😃 .
Since NNBD, [dart-lang/collection] have recently added Iterable#firstOrNull.

[hoc081098]

I just open PR #560, works as expected

[larssn]

Your example only works when you pass a seed value

Yeah it's quirky. I'm gonna raise an issue with the Dart team about it. I'm sure they discussed it, and I'm interested in the outcome.

, but the whole complexity lies with value being not nullable, but also not being seeded,

Not nullable AND not seeded? Ergo null? 😅

Let me ask you: Is it logical to support a field that is non-nullable, but should be able to store nulls (as it would if not seeded). Dart will NEVER allow that to happen. No matter how many custom errors or new fields you dream up, that is just not possible. And even if you find some kind of workaround, is that something you want? It clearly goes against the whole point of NNBD. 🙂

If you check BehaviorSubject sources for example, you can see how cumbersome it is to support it.

Then don't. What is gained from it? The consumer should be able to inject their own types.

[frankpepermans]

Your example only works when you pass a seed value

Yeah it's quirky. I'm gonna raise an issue with the Dart team about it. I'm sure they discussed it, and I'm interested in the outcome.

, but the whole complexity lies with value being not nullable, but also not being seeded,

Not nullable AND not seeded? Ergo null? 😅

Let me ask you: Is it logical to support a field that is non-nullable, but should be able to store nulls (as it would if not seeded). Dart will NEVER allow that to happen. No matter how many custom errors or new fields you dream up, that is just not possible. And even if you find some kind of workaround, is that something you want? It clearly goes against the whole point of NNBD. 🙂

If you check BehaviorSubject sources for example, you can see how cumbersome it is to support it.

Then don't. What is gained from it? The consumer should be able to inject their own types.

There's a solution for that in nnbd, it's called late and IMO make perfect sense here,
value will be set at some point, and is non-nullable, ergo it's late, and we should expose it like that as well.

[larssn]

But I don't think late is not a good match here. late should be used when you can guarantee that the variable is initialized shortly after the constructor, which one cannot do here. It might be null forever.

I feel I'm failing in convincing you that this is not a good idea. I can live with a valueOrNull field, even though value also might be null, and thus having two identical fields.

[frankpepermans]

I meant as an analogy to late,
late as in, you are sure that it exists by now, and you access value, but if it should still be null however, then it throws.

Therefore, valueOrNull is a valid addition, and is allowed to be T?.

For me, it makes most sense that value is the exact same Type as the Stream Type, otherwise you might leak Type errors after consuming value in your code.

[larssn]

For me, it makes most sense that value is the exact same Type as the Stream Type, otherwise you might leak Type errors after consuming value in your code.

The whole type system is there to prevent leaks. That shouldn't happen.
If you take my simple example above, and try to do:

final nullable = SimpleBehaviorSubject<num?>(1);
print(nullable._val.toDouble()); // <-- Dart will not allow this to compile, with the error: The method 'toDouble' can't be unconditionally invoked because the receiver can be 'null'.

Having T be non-nullable is just not possible unless you seed it.

So if we follow the idioms (and we should) set forth by static analysis we get:
Unseeded and the type is T?, seeded and the type is T.

I'd love to be wrong on that!

ValueStream is a weird beast to begin with: it promises (or tries to) that a value is available outside the context of the stream. It cannot ever make that promise (unless seeded).

[frankpepermans]

The point is, that BehaviorSubject<int>() for example is totally valid, there is no reason why it should be int? at all.
otherwise, you'd be able to do subject.add(null).

Value exists because you want to peek at the current, last emitted value, which is expected to be of type int.
It's unsafe by nature as there is no guarantee it is filled at the time of accessing it.

Take Stream.first, Stream.last for example, if the Stream closes before any event was added, then this will throw an Exception.
value could be analogous to the above as Stream.current, but if it has not emitted yet, like above, then also throw an Exception.

[larssn]

The point is, that BehaviorSubject<int>() for example is totally valid, there is no reason why it should be int? at all.
otherwise, you'd be able to do subject.add(null).

Yeah that definitely should not be possible.

Value exists because you want to peek at the current, last emitted value, which is expected to be of type int.
It's unsafe by nature as there is no guarantee it is filled at the time of accessing it.

Take Stream.first, Stream.last for example, if the Stream closes before any event was added, then this will throw an Exception.
value could be analogous to the above as Stream.current, but if it has not emitted yet, like above, then also throw an Exception.

But I don't see a problem with value being nullable if not seeded. If not seeded I'd LIKE the type system to warn me about it: "Hey you, value might be null because the stream might not have emitted anything yet".

I'd rather have a null (and the full help of the type system) than an exception thrown in my face which is:

1. More cumbersome to handle.
2. The type system is no help at all when it comes to exceptions (stating the obvious).

EDIT:
A Dart dev shed light on explicitly passing null to nullable params:
dart-lang/sdk#45256 (comment)