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

Order of subscribeBy parameters is not the same as standard RxJava's subscribe #174

Open
BoD opened this issue Mar 3, 2018 · 8 comments

Comments

@BoD
Copy link

BoD commented Mar 3, 2018

With standard RxJava, you can do:

mySingle
    .subscribe ({ result -> println(result) }, {e -> e.printStackTrace()})

so one would suppose that with RxKotlin you could do

mySingle
    .subscribeBy({ println(it) }, { it.printStackTrace() })

but in fact, this is not possible, because the first parameter is onError, and the second one is onSuccess, which is the opposite of the order you have on RxJava's subscribe.

Because of this, you are forced to use named parameters like this...

mySingle
    .subscribeBy(onSuccess ={ println(it) }, onError={ it.printStackTrace() })

That's too bad :(

But it's even more confusing if you pass only one parameter. One would expect that it would be onSuccess (line standard RxJava method that takes only one parameter), but in fact it's onError, which is very counter intuitive and surprising.

@thomasnield
Copy link
Collaborator

I think I reversed the order for that reason that passing one nameless parameter attached to the onSuccess(). Now if this isn't the case anymore, it needs to be investigated and possibly changed.

@JakeWharton
Copy link

JakeWharton commented Mar 7, 2018 via email

@BoD
Copy link
Author

BoD commented Mar 7, 2018

I'm sorry, I don't understand this sentence "I think I reversed the order for that reason that passing one nameless parameter attached to the onSuccess()."

@thomasnield
Copy link
Collaborator

@BoD Try this and you should see it works with the current setup:

mySingle.subscribeBy { result -> println(result) }

@JakeWharton and yes, overloads are probably the most effective way to fix this. Accepting PR's from anyone as I'm kind of busy right now.

@BoD
Copy link
Author

BoD commented Mar 10, 2018

You are right, it works indeed, so the last part of what I wrote is incorrect. I can't figure out right now what I did that made me think the single parameter was onError... Weird!

@grabstepango
Copy link

Don't think there is any problem here. Passing multiple lambdas in one function would any way benefits from named parameters.

@arekolek
Copy link
Contributor

arekolek commented Jul 13, 2018

You are right, it works indeed, so the last part of what I wrote is incorrect. I can't figure out right now what I did that made me think the single parameter was onError... Weird!

@BoD Because

mySingle.subscribeBy { result -> println(result) }

is not the same as

mySingle.subscribeBy({ result -> println(result) })

In Kotlin, there is a convention that if the last parameter of a function accepts a function, a lambda expression that is passed as the corresponding argument can be placed outside the parentheses

I thought the order of arguments of subscribeBy was intentional, because it's convenient to do stuff like:

mySingle.subscribeBy(Timber::e) { result -> println(result) }

@omeryounus
Copy link

Umm I don't understand how these working, I never found onError throwing any errors. Its seems like its just there for no reason

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

No branches or pull requests

6 participants