Skip to content
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

Add combineLatest overload for Collection #3660

Closed
wants to merge 1 commit into from
Closed

Add combineLatest overload for Collection #3660

wants to merge 1 commit into from

Conversation

JakeWharton
Copy link
Contributor

2.x already uses Iterable, but that's a very drastic change compared to just Collection.

Motivation here is that I'm using set bindings to create observables in a dependency injector and I want to skip the current new ArrayList<>(sources) that I have to do on the set.

@akarnokd
Copy link
Member

akarnokd commented Feb 1, 2016

Can we revisit this once #3507 is merged?

@JakeWharton
Copy link
Contributor Author

Yep. That allows exposing Iterable as well which is even better.

* Observables by means of the given aggregation function
* @see <a href="http://reactivex.io/documentation/operators/combinelatest.html">ReactiveX operators documentation: CombineLatest</a>
*/
public static <T, R> Observable<R> combineLatest(Collection<? extends Observable<? extends T>> sources, FuncN<? extends R> combineFunction) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

final? :trollface:

@artem-zinnatullin
Copy link
Contributor

👍

Don't you want to add tests for your new Observable.combineLatest()? OnSubscribeCombineLatest.java is internal, will be great to cover public API as well for possible changes in future implementation and to test operator from user's point of view.

@JakeWharton
Copy link
Contributor Author

The tests were modified to use it.

On Mon, Feb 1, 2016 at 10:27 AM Artem Zinnatullin [email protected]
wrote:

[image: 👍]

Don't you want to add tests for your new Observable.combineLatest()?
OnSubscribeCombineLatest.java is internal, will be great to cover public
API as well for possible changes in future implementation and to test
operator from user's point of view.


Reply to this email directly or view it on GitHub
#3660 (comment).

@artem-zinnatullin
Copy link
Contributor

Tests for OnSubscribeCombineLatest were modified, but I'm talking about tests for Observable.combineLatest(Collection, FuncN).

@JakeWharton
Copy link
Contributor Author

They use that API.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants