Reactive: Could we have Tuple Overload for Zip, CombineLatest, and any other combine type method?

[Thaina]

Feature request

It would be more comfortable to work with Zip and CombineLatest to let it generate Tuple by default when we don't supply result function

C# IObservable<L> lStream; IObservable<R> rStream; lStream.Zip(rStream).Do((pair) => Console.WriteLine(pair)); // pair is ValueTuple<L,R>

Is it possible?

[quinmars]

Since there is a IEnumerable<> counterpart in dotnetcore for that, it is probably a good idea to add at least the Zip variant to keep symmetry with IEnumerable<>.

[bartdesmet]

Agreed. The symmetry is worth having and extending to CombineLatest and WithLatestFrom as well. We're happy to take a community contribution for this. It'd have to be complete though, i.e.:

• For Zip, CombineLatest, and WithLatestFrom.
• All overloads are provided.
  
  
  
  • E.g. Zip has 16. So tuple names would be First, Second, ..., Sixteenth.
  
  
  • There's also a Zip overload that has IObservable<> and IEnumerable<>.
  
  
• Also added in Qbservable to keep parity with Observable.

Example:

public static IObservable<(TFirst First, TSecond Second)> Zip<TFirst, TSecond> (this IObservable<TFirst> first, IObservable<TSecond> second);

We have to match the casing (First rather than first) and use tuple syntax (rather than ValueTuple<...> types directly), so the component names flow.

Also, right now, these methods also use TSourceN where N varies from 1 to 16. Because names for tuple-based overloads leak into the tuple component names, we'll have to use the proper English ordinals, prefixed with T for the generic parameters.

One caveat I can see at this point is that we'll need some #if in the code because ValueTuple<> isn't supported in all platforms Rx is still targeting today. @clairernovotny, any thoughts on our target platforms strategy? I'm happy to add more feature flags in the .targets file (i.e. HAS_VALUE_TUPLE) but we may need to refine the platform matrix a little bit because there's e.g. .NET 4.6 but no .NET 4.7 (which has these types). For this feature to be enabled we'd need any of these:

• .NET Framework 4.7 or above
• .NET Standard 2.0 or above
• .NET Core 1.0 or above
• .NET 5 (or above)

Maybe, once .NET 5 ships, we can tighten the target platforms matrix a little bit? (E.g. I'd be happy to boost .NET Framework minimum requirements for Rx vNext to v4.7.2 where a lot of issues were addressed IIRC.)

[bartdesmet]

Preliminary work to make adding overloads easier is done in https://github.com/dotnet/reactive/pull/1302.

[bartdesmet]

There's an unfortunate breaking change that needs to be pondered before we go ahead:

public static IObservable<IList<TSource>> Zip<TSource>(params IObservable<TSource>[] sources)

If an overload is added for tuples, such as:

public static IObservable<(TFirst First, TSecond Second)> Zip<TFirst, TSecond>(this IObservable<TFirst> first, IObservable<TSecond> second)

then code like this (where xs and ys have the same element type):

Observable.Zip(xs, ys)

encounters a change in return type (from IObservable<IList<T>> to IObservable<(T, T)>), thus breaking user code.

The same issue occurs for CombineLatest.

[clairernovotny]

Now is a good time for a breaking change. I bumped the ver to 5.x in #1291 since I had to also rename DispatchObservable to CoreDispatchObservable (along with ScheduleOnDispatcher -> ScheduleOnCoreDispatcher and ObserveOnDispatcher -> ObserveOnCoreDispatcher) since .NET 5.0-windows supports both in the same project.

If we need to do more breaking changes, I think it's a good time to do it.

[bartdesmet]

We can't really take away the params-based overload; there is no good alternative for it. Anyone using this overload (that's been there since day 1) will have been writing Observable.Zip(xs, ys), for any number of sources, and always get an IObservable of lists. When adding these new variants, the old behavior would show up when specifying more than 16 sources (or whatever number of overloads we support), but not when specifying less than 16.

The only alternative I see right now is to put the new overloads in ObservableEx (or whatever name). They're still discoverable as extension methods. But Observable.Zip won't bind to the new overloads, this preserving compat.

Nothing is more frustrating than upgrading a dependency and seeing your code (possibly more than 8 years old in this case) break in a random place. It's twice as bad if there's no alternative to restore the old behavior. So let's proceed with caution and put them on a different type. Still extension methods, so most people won't notice. Only when calling them as static methods does the ObservableEx (or whatever) type show up.

[bartdesmet]

I'll prepare a PR later this weekend, time permitting.

[bartdesmet]

This is checked in now.