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

2.x: Fix Generics T[] in Zip & CombineLatest #4525

Merged
merged 5 commits into from
Sep 10, 2016
Merged

2.x: Fix Generics T[] in Zip & CombineLatest #4525

merged 5 commits into from
Sep 10, 2016

Conversation

vanniktech
Copy link
Collaborator

@vanniktech vanniktech commented Sep 10, 2016

Fixes #4524

Test for combineLatest fill follow

@Test
public void zipIterableObject() {
final List<Flowable<Integer>> flowables = Arrays.asList(Flowable.just(1, 2, 3), Flowable.just(1, 2, 3));
Flowable.zip(flowables, new Function<Object[], Object>() {
Copy link
Member

Choose a reason for hiding this comment

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

Yeah, this always passes because it is compatible with both ? super T[] and ? super Object[]. I have no idea how to test with Function<Integer[], Object> because it doesn't even compile with the latter signature. Leave this as is.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yup Function<Integer[], Object> didn't work for me so I didn't add a test case.

@@ -194,7 +194,7 @@ public static int bufferSize() {
*/
@SchedulerSupport(SchedulerSupport.NONE)
@BackpressureSupport(BackpressureKind.FULL)
public static <T, R> Flowable<R> combineLatest(Function<? super T[], ? extends R> combiner, Publisher<? extends T>... sources) {
public static <T, R> Flowable<R> combineLatest(Function<? super Object[], ? extends R> combiner, Publisher<? extends T>... sources) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

this one does not seem necessary to be honest. It just has switched arguments with the one above. Or do I oversee anything?

Copy link
Member

Choose a reason for hiding this comment

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

This and other overloads of operators let's you use varargs to specify any number of sources at the expense that varargs has to be the last argument.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ups right didn't see those little dots there too many overloads ...

@akarnokd akarnokd added the Bug label Sep 10, 2016
@akarnokd akarnokd added this to the 2.0 RC 3 milestone Sep 10, 2016
@codecov-io
Copy link

codecov-io commented Sep 10, 2016

Current coverage is 78.71% (diff: 100%)

Merging #4525 into 2.x will increase coverage by <.01%

@@                2.x      #4525   diff @@
==========================================
  Files           515        515          
  Lines         34643      34644     +1   
  Methods           0          0          
  Messages          0          0          
  Branches       5431       5431          
==========================================
+ Hits          27268      27269     +1   
+ Misses         5409       5408     -1   
- Partials       1966       1967     +1   

Powered by Codecov. Last update 44c5705...4a972f2

@vanniktech
Copy link
Collaborator Author

This should be it now

@akarnokd
Copy link
Member

Single has exactly 1 and Maybe at most 1 item, so there is only 1 latest that can happen thus there is no difference between combineLatest and zip for these sources.

@akarnokd
Copy link
Member

👍

@vanniktech
Copy link
Collaborator Author

Yup I immediately noticed it after I wrote my comment :D that's also why I deleted it

@akarnokd akarnokd merged commit 9966209 into ReactiveX:2.x Sep 10, 2016
@vanniktech vanniktech deleted the 2.x_generics_object_fix branch September 10, 2016 12:15
@akarnokd akarnokd mentioned this pull request Sep 21, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants