search

raquo / Airstream

State propagation and event streams with mandatory ownership and no glitches
MIT License
246 stars 28 forks source link

Add method variant of EvenStream.merge? #97

Closed fgoepel closed 1 year ago

fgoepel commented 2 years ago

Maybe that's just me, but it took me quite a while to discover its existence, as I was expecting there to be some kind of 'concat' or 'merge' method on EventStream (maybe even a '++' alias).

Is there a reason why this doesn't exist?

raquo commented 2 years ago

Hey, sorry, somehow I missed this issue's notification! Getting back to working on 15.0.0 now, and just saw it.

Yep I can add a mergeWith operator on streams, not sure why I didn't do that from the start tbh.

Re: naming – concat is typically associated with different semantics in observables so I can't use that for merging. Kinda same for ++ since it's usually associated with concat, plus I'm not a fan for symbolic names in observables – there's just too many ways to combine two streams for ++ to have an obvious meaning (the same is not true for e.g. the collections library because it does not deal with time).

fgoepel commented 2 years ago

No worries.

Having a mergeWith method sounds good to me. It seems it's called merge in Monix Observable, fs2, and ZIO streams (although I didn't check whether the semantics match exactly), but you may have reasons to diverge.

raquo commented 2 years ago

I'm following the naming of our combineWith for consistency. That one just happens to be historical already, I don't think one or the other name is fundamentally better, just don't want to make our big video out of date (text is easy to update, video – not so much). IDE's autocomplete should expose both names equally well anyway.

fgoepel commented 2 years ago

Fair enough.

raquo commented 2 years ago

Makes sense, I'll add that method. Not sure why I didn't do that to start with tbh.

Message ID: @.***>

--Nikita