Closed polytypic closed 8 years ago
The Rx team fixed the issue with respect to the StackOverflowException
in Rx 2.0. However, you could certainly be correct. I think it would be helpful to create some tests to show how this could work better. Also, I believe we implement Delay
, which would mean the Combine
implementation is wrong in any case, since it should at least be (I think):
member __.Combine(comp1, comp2) = Observable.Concat(comp1, comp2())
That said, Delay
is not implemented in the typical way and is also likely incorrect.
Another aspect we should reconsider: Why do we have both Return
and Yield
? As Observable
s are sequence-like, they should probably implement Yield
only and avoid Return
. I don't think this really hurts anything, but it would have been nice to either differentiate them (e.g. Return
enforces a single result) or not implement both operators.
FYI, I've been thinking about this a bit further and I came up with the following experimental approach for choice streams: Builder. In other words, I think that depending on the desired results, more than one of the "join" operations may be appropriate. I think that append
(or concat) and switch
, in particular, are useful join operations on streams. Append corresponds to collecting all possible results sequentially. It is like collect
on sequences. switch
, on the other hand, corresponds to the idea that one wants to compute the latest value depending on some changing inputs. It is roughly what UI.Next
does. I think that merge
, corresponding to Merge
and SelectMany
of Rx, is not that useful or practical. I think that the emphasis on SelectMany
in Rx may be a historical mistake.
Yes, I agree that yield
is preferable to return
. Delay
and Combine
seem fine to me as they are, but I might be mistaken as I have not yet found a good definition of the semantics of Rx (cue @headinthebox). Using
I'm not sure about. If it returns the same value res
multiple times, then will it be disposed multiple times and might the value be used after it has been disposed?
Looking at this a bit more, I'm not sure I agree that SelectMany
should be replaced by Select
+ Concat
. While that would probably work just fine, I would expect Bind
to behave in according to Merge
and not Concat
. However, with respect to Combine
, Concat
makes sense as I would have specified and expected the first to complete before running the latter. I'm still open to discussing further, of course, and still thinking about the ramifications of the changes.
Sorry for the previous reply—I was in the wrong monad. :)
I think one question to ask here is whether given monadic Join
member builder.Join (xMM: M<M<'x>>) : M<'x> = builder.Bind (id, xMM)
the expression
builder.Join (builder.Return (builder.Combine (xs, ys)))
and the expression
builder.Join (builder.Combine (builder.Return xs, builder.Return ys))
should have the same meaning?
The above holds when the same form of composition (Addition: Concat
or Merge
) is used in both Combine
and Bind
and otherwise does not.
However, it may well be that there are many different useful combinations of semantics for Combine
and Bind
with intuitive meanings. Concat
and Merge
is not the only useful one and perhaps not even the most useful or intuitive one.
@VesaKarvonen, what do you suggest?
I was mainly just pointing out something that, IMHO, needs careful consideration. I don't have a specific suggestion for what you should do. So, feel free to close this issue.
Rx streams can be used for a variety of purposes. The particular combination of Merge
and Concat
that the ObservableBuilder
uses doesn't necessarily cover all of those and may also produce unexpected results as Combine
doesn't distribute under Join
like with ordinary sequences (the question raised in my previous reply).
Also note that using sequential Concat
semantics for Combine
, For
and While
in ObservableBuilder
does not guarantee that mutable state created within the builder could not be accessed in parallel, because using Merge
semantics for Bind
introduces the possibility of parallel execution.
In my choice stream library, I'm currently providing four separate builders:
/// This builder joins substreams with `append` to produce a stream with all
/// results in sequential order.
val appended: Builder
/// This builder joins substreams with `merge` to produce a stream with all
/// results in completion order.
val merged: Builder
/// This builder joins substreams with `amb` to produce a stream with the
/// first results.
val ambed: Builder
/// This builder joins substreams with `switch` to produce a stream with the
/// latest results.
val switched: Builder
I think that these correspond to four particular purposes for which one might use choice streams.
The first two are cases where one uses choice streams to get a collection of all results. The difference between them is that the first one, appended
, collects the results sequentially in query order while the second one, merged
, collects the results in parallel in completion order. appended
is basically the same thing one would get with ordinary sequences and one could use it when the order of results is important. merged
, on the other hand, is useful when the order of results is not important, but you still want a collection of all the results.
The two remaining are cases where one doesn't use choice streams to get a collection of all results, but a result, or an ordered sequence of results, that are either the first results or the latest results. With ambed
you probably have a situation where you have multiple data sources each producing the same or equivalent result and you just want any one of them as quickly as possible. With switched
you probably have a situation where you want to compute an up-to-date result that depends on various time varying properties. Each time a particular property changes, you are no longer interested in results based on old values of that property.
These four builders probably don't cover all of the use cases. As all of the builders create choice streams, one could use two or more of the builders to create the desired combination of semantics.
That is very enlightening! Thank you for the reply. I'm not sure how to proceed either but will leave this open for further discussion. Perhaps @headinthebox will finally weigh in at some point.
I'm going to leave as-is for now. I would appreciate additional feedback. Also, @polytypic, if you need this change made for a specific use case, please let me know, as that will add more weight to the request.
I just looked at the
ObservableBuilder
for the first time and noticed it has the following definitions:SelectMany
merges the observable sequences, whileConcat
concatenates the sequences. To me this looks very odd, because this means the builder uses two different semantics for combining sequences. Googling revealed that this aspect has also been noticed earlier.To me it would make more sense if
Bind
were defined as:Note that I just mention this because the current definition looks odd. I have no plans to use the builder at the moment.