-
Notifications
You must be signed in to change notification settings - Fork 7.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Added support for custom functions in combineLatest. #763
Conversation
RxJava-pull-requests #676 SUCCESS |
I think the same can be achieved by calling map over standard combineLatest, no need for a special method. |
To illustrate, that's how I do it in my akka with rx monitoring project: |
I agree with @chrisgrimm that this one should be there. This one is not very handy (and less performant) since in 99% of the case you will immediately destruct the tuple by applying a function. @benjchristensen can you merge this?
|
@headinthebox @chrisgrimm yes, the proposed one is more general and doesn't lose performance in 99% of the cases. Maybe the existing one should be removed then? |
I guess it is the idiomatic Scala way, like Zip, so I think we should keep it just for consistency. |
My thinking was in line with @headinthebox . The other issue I'd like to see tackled with with the combineLatest function is the ability to zip many Observables together. The current implementation can only zip two things together which makes it hairy to handle larger zippings. You get this kind of "recursive Tuple2" thing going on if you repeatedly zip things, which can be resolved in Scala (http://stackoverflow.com/questions/21154639/higher-order-operations-with-flattened-tuples-in-scala), but it is annoying nonetheless. |
You may want to look at the join patterns. On Thu, Jan 23, 2014 at 11:30 PM, Chris Grimm [email protected]:
|
@headinthebox @chrisgrimm right, I see you want to keep the API consistent, though not minimal. To do so however, the current implementation of combineLatest should also be changed to delegate to the new one like it's done in zip. |
Correct. Send a pull request :-) On Fri, Jan 24, 2014 at 12:54 PM, Eugene Vigdorchik <
|
@headinthebox sure, if @chrisgrimm doesn't beat me to it :) |
@chrisgrimm Unfortunately I don't see your fork in the list of forks of Netflix/RxJava, so I can't send a PR to it. |
Added support for custom functions in combineLatest.
No description provided.