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

Sam Adapters for RxJava 2.x #114

Merged
merged 7 commits into from May 2, 2017
Merged

Sam Adapters for RxJava 2.x #114

merged 7 commits into from May 2, 2017

Conversation

thomasnield
Copy link
Collaborator

This is the SAM adapter implementation WIP for RxKotlin 2.x, initiated from the following discussions by @chris-horner:

#103
#109

Add more zip() and combineLatest() functions to ObservableKt
add more zip() and combineLatest() to FlowableKt
@thomasnield
Copy link
Collaborator Author

I will look into Travis build errors later. For now, I see we need to create SAM adapters for the following operators, for ObservableKt, FlowableKt, MaybeKt, and SingleKt where applicable:

  • ObservableKt.combineLatest()
  • ObservableKt.zip()
  • Observable#zipWith()
  • Observable#withLatestFrom()

There might be some needed for flatMap() and concatMap() overloads as well. Please let me know if I am missing any others.

@thomasnield
Copy link
Collaborator Author

Not quite sure why the Kotlin compiler is not liking ObservableKt, FlowableKt, SingleKt, and MaybeKt. It says these are duplicate JVM classes. Looking through the .class files and I'm not quite sure if I see any duplicates. I really like those names and want them to work. I'd rather change what they are clashing with.

image

@thomasnield
Copy link
Collaborator Author

Renamed ObservableKt to Observables, FlowableKt to Flowables, etc... to resolve build issues at the moment.


object Flowables {

inline fun <T1,T2,R> combineLatest(source1: Flowable<T1>, source2: Flowable<T2>, crossinline combineFunction: (T1, T2) -> R) =
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: I know people aren't often going to be reading this file, but I feel combiner is a nicer name than combineFunction.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Okay Ill change that.


inline fun <T1,T2,T3,R> combineLatest(source1: Flowable<T1>, source2: Flowable<T2>, source3: Flowable<T3>, crossinline combiner: (T1,T2, T3) -> R) =
Flowable.combineLatest(source1, source2,source3,
io.reactivex.functions.Function3<T1, T2, T3, R> { t1: T1, t2: T2, t3: T3 -> combiner(t1,t2, t3) })!!
Copy link
Contributor

Choose a reason for hiding this comment

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

Any particular reason for the fully qualified Functions?

@thomasnield
Copy link
Collaborator Author

My compiler struggled if I didn't.

@chris-horner
Copy link
Contributor

Have a look at the methods I've defined that you've copied across. Function3, 4, and 5` all work without being fully qualified. Perhaps IntelliJ's not grabbing the correct import for you?

@thomasnield
Copy link
Collaborator Author

I'll follow up this weekend on this.

@thomasnield
Copy link
Collaborator Author

@chris-horner Couldn't figure out the BiFunction not inferring correctly, but was able to clean up the others.

# Conflicts:
#	src/main/kotlin/io/reactivex/rxkotlin/Flowables.kt
#	src/main/kotlin/io/reactivex/rxkotlin/Observables.kt
@@ -1,5 +1,5 @@
buildscript {
ext.kotlin_version = '1.1.0'
ext.kotlin_version = '1.1.2'
Copy link
Collaborator

Choose a reason for hiding this comment

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

1.1.2-2 is the latest one

package io.reactivex.rxkotlin

import io.reactivex.Flowable
import io.reactivex.functions.*
Copy link
Collaborator

Choose a reason for hiding this comment

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

avoid wildcard imports

@@ -0,0 +1,144 @@
package io.reactivex.rxkotlin
Copy link
Collaborator

Choose a reason for hiding this comment

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

code formatting in all file


inline fun <T1,T2,R> combineLatest(source1: Flowable<T1>, source2: Flowable<T2>, crossinline combineFunction: (T1, T2) -> R) =
Flowable.combineLatest(source1, source2,
BiFunction<T1, T2, R> { t1, t2 -> combineFunction(t1,t2) })!!
Copy link
Collaborator

Choose a reason for hiding this comment

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

why !!?

@thomasnield thomasnield merged commit 85f31eb into 2.x May 2, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants