ashdavies / rx-tasks

Simple and lightweight RxJava2 wrapper for the GMS Tasks API
Apache License 2.0
65 stars 15 forks source link

Allow specifying the scheduler #16

Closed bensandee closed 6 years ago

bensandee commented 6 years ago

Allow specifying the scheduler when observing task results to directly receive task results on thread other than main thread.

ashdavies commented 6 years ago

I really wanted to avoid providing optional configuration parameters and default fallbacks with nullability, and given that this can be achieved with the consumer, it makes this just a convenience that would risk muddying up the implementation.

bensandee commented 6 years ago

I certainly understand the desire to keep the API surface area lean, but using observeOn simply masks the issue and doesn't prevent the results from being dropped onto the main thread first. To call it a convenience isn't really accurate. One could argue whether there's sufficient performance need for these overloads in any given situation, but really that should be up to each app developer to decide.

No worries though, I'll maintain my fork over at https://github.com/tbsandee/rx-tasks for now in case anyone else needs the overloads.

ashdavies commented 6 years ago

I was taking another look at this, and reading into the documentation for the Tasks API, which does actually offer an option for an Executor which this library ignores (I was previously of the opinion it would be duplicating observeOn), thus it would make sense to provide this functionality.

That being said, I'm not happy with including nullability, and I'd like to consider an alternative approach, perhaps with a default parameter or overloaded API. I'll create a new issue for this.

bensandee commented 6 years ago

My nullability fix was there to support when the Tasks API emits null values (allowed) and the user uses a Completable. It doesn't propagate null values into the Rx stream, nor should it.

ashdavies commented 6 years ago

I'm referring to the Scheduler being a nullable val

bensandee commented 6 years ago

Fair enough, it's your party.

ashdavies commented 5 years ago

This should be available with version 2.2.0