Skip to content

Conversation

@stepango
Copy link
Collaborator

@stepango stepango commented Jan 7, 2017

Start working on rxKotlin for rxJava2. For now, it's just a raw port of rxKotlin. Any feedback highly appreciated.

@stepango stepango mentioned this pull request Jan 7, 2017
@thomasnield
Copy link
Collaborator

Awesome! I'll take a look later...

@stepango
Copy link
Collaborator Author

stepango commented Jan 7, 2017

@thomasnield thanks a lot

LICENSE Outdated
To apply the Apache License to your work, attach the following
boilerplate notice, with the fields enclosed by brackets "[]"
replaced with your own identifying information. (Don't include
replaced with your own identifying information. (Don'value include
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Keep Don't

build.gradle Outdated

task wrapper(type: Wrapper) {
gradleVersion = '2.2.1'
gradleVersion = '2.10'
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Any chance for nebula to work with gradle 3+?

Choose a reason for hiding this comment

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

In RxAndroid we dropped nebula. It hurts more than it helps.

subscriber.onNext("Hello")
subscriber.onCompleted()
}).subscribe { result ->
Observable.create<kotlin.String> { onSubscribe ->
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

kotlin.String -> String

build.gradle Outdated

task wrapper(type: Wrapper) {
gradleVersion = '2.2.1'
gradleVersion = '2.10'

Choose a reason for hiding this comment

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

In RxAndroid we dropped nebula. It hurts more than it helps.

println("--- Error ---\n${e.message}")
}

Choose a reason for hiding this comment

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

What's with all the added trailing whitespace?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Looks like default kotlin formatting settings in one of the last plugin releases.

fun asyncWiki(vararg articleNames: String): Observable<String> = observable { subscriber ->
thread {
articleNames.toObservable()
.flatMapMaybe { name: String -> URL("http://en.wikipedia.org/wiki/$name").toScannerObservable().firstElement() }

Choose a reason for hiding this comment

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

Does the argument need an explicit type?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Nope

/**
* [Observable.defer] alias
*/
fun <T : Any> Observable<T>.defer() = Observable.defer { this }

Choose a reason for hiding this comment

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

This shouldn't exist. The whole point of defer is the function is deferred to create an Observable. If you already have one there's no point in calling defer.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Agreed, forgot to delete it before commit


@Suppress("BASE_WITH_NULLABLE_UPPER_BOUND") fun <T> Observable<T>.onErrorReturnNull() : Observable<T?> = onErrorReturn<T> {null}
fun <T : Any> T.toSingletonObservable(): Observable<T> = Observable.just(this)
fun <T : Any> Throwable.toObservable(): Observable<T> = Observable.error(this)

Choose a reason for hiding this comment

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

These two are a bit ridiculous

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yes

Code review related fixes.
@stepango stepango self-assigned this Jan 8, 2017
import io.reactivex.disposables.Disposable
import java.util.ArrayList

class FunctionSubscriber<T : Any> : Observer<T>, MaybeObserver<T>, SingleObserver<T>, CompletableObserver {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Reimplement using OnSubscribe* interfaces

}

private fun URL.toScannerObservable() = observable<String> { s ->
private fun URL.toScannerObservable() = observable<String>({ s ->
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

remove ()

@stepango
Copy link
Collaborator Author

Porting subscribeWith was a good exercise but looks like i'll drop it from rxKotlin 2.x and rxKorlin 1.x as well. Probably release it as a separate artefact later.

@thomasnield
Copy link
Collaborator

I was thinking about that for a few days. Makes sense but it's too bad. Is there a way to present each action as a named parameter to subscribe()?

@thomasnield
Copy link
Collaborator

import io.reactivex.Observable

fun main(args: Array<String>) {

    Observable.just("Alpha", "Beta", "Gamma")
            .subscribe(onNext = ::println, onComplete = { println("done") })

}

fun <T> Observable<T>.subscribe(onNext: ((T) -> Unit)? = null,
                                onError: ((Throwable) -> Unit)? = null,
                                onComplete: (() -> Unit)? = null) = subscribe(onNext, onError, onComplete)

@stepango
Copy link
Collaborator Author

@thomasnield that's exactly what I'm thinking about. The only problem is - the name of a function, ide will not auto-import it because of the name clash. subscribeWith is taken already and has different semantics in rxJava2. As an option, function could be named subscribeBy. Any ideas?

@stepango
Copy link
Collaborator Author

RxJava2 don't accept null params in subscribe()

@thomasnield
Copy link
Collaborator

RxJava2 doesn't accept null anything it seems. I guess you can just create a hard observer in the implementation then...

flowable extensions added
minor fixes
@iNoles
Copy link

iNoles commented Feb 11, 2017

@stepango According to https://kotlinlang.org/docs/reference/packages.html, you can use alias for name clash such as like "import bar.Bar as bBar // bBar stands for 'bar.Bar'"

@stepango
Copy link
Collaborator Author

@iNoles I know about import aliases, do you have some particular use case in mind?

@iNoles
Copy link

iNoles commented Feb 12, 2017

@stepango I was thinking about to use rxjava as import alias.

@stepango
Copy link
Collaborator Author

@iNoles don't get it. What exactly do you mean? Any code examples?

@thomasnield
Copy link
Collaborator

@stepango is it possible you can merge what you have into 2.x? I'd like to start contributing to this and it might be easier to centralize it into the repository.

@stepango
Copy link
Collaborator Author

@thomasnield was just about to do that. Finally, have some free time this weekend.

@stepango stepango merged commit 2a6561c into ReactiveX:2.x Feb 25, 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.

4 participants