https://kotlinlang.org logo
#android
Title
# android
z

zak.taccardi

04/13/2020, 6:18 PM
What’s the proper way to inject the
CoroutineDispatcher
for
LifecycleCoroutineScope
in production? Seems it’s hardcoded to
Dispatchers.Main.immediate
and
SupervisorJob
which is something I desire flexibility around https://developer.android.com/topic/libraries/architecture/coroutines#lifecyclescope
w

wasyl

04/13/2020, 6:24 PM
If you need that flexibility, maybe you don’t need the built in scope? What’s the scenario you need this for, out of curiosity?
z

zak.taccardi

04/13/2020, 6:24 PM
we provide the ability to inject the dispatchers
so directly leveraging
Dispatchers.Main.immediate
is a violation of that
w

wasyl

04/13/2020, 6:26 PM
we provide the ability to inject the dispatchers
Yes but why 😄
I mean, if you need that much control I suppose your best bet is to you have your own scope. Changing the one provided by Arch doesn’t seem like a good idea
maybe I want to inject a specific
CoroutineName
for debugging purposes, who knows
f

fatih

04/13/2020, 6:34 PM
You can inject dispatcher with creating custom annotation for each dispatcher and providing it if you are using dagger
👍 1
z

zak.taccardi

04/13/2020, 6:35 PM
if you are using dagger
no
f

fatih

04/13/2020, 6:35 PM
Ok :)
z

zak.taccardi

04/13/2020, 6:36 PM
creating custom annotation for each dispatcher
Not using reflection, java 8, or annotation processing
w

wasyl

04/13/2020, 6:37 PM
z

zak.taccardi

04/13/2020, 6:38 PM
setting static state has side effects, which makes any solution that leverages that unacceptable. For an individual fragment, I want to inject which dispatcher it uses. I don’t want to override a static one
It looks to me that the AndroidX team failed in this regard and has designed their code here in a way to prevent dependency injection and instead rely on static state and all of its problems
w

wasyl

04/13/2020, 6:42 PM
😄 Then why use provided view model extensions instead of writing your own?
z

zak.taccardi

04/13/2020, 6:49 PM
once you have a reference to a
Fragment
, you can inject your own
ViewModel
.viewModelScope
is an anti-pattern because it prevents injecting a
CoroutineScope
, so I don’t use it
like the workaround for enabling
viewModelScope
to work when under test is a joke
but all the functions on
LifecycleScope
are very useful, like
launchWhenStarted
r

rkeazor

04/14/2020, 6:26 AM
I think for most usecases viewModelScope ,will probably be enough.. its already tied to the lifecycle of the viewModel which is already lifecycle aware... As for testing purposes, one way to make testing easier is to create and utilize CoroutineScope extenstion functions or use coroutineScope Builder function. That way when you use runBlocking in your Tests, it always inherits the test scope... Or if you do need to create randomly excute a job withIn a function , just Dispatchers.setMain(...) works well with viewModelScope
z

zak.taccardi

04/14/2020, 2:33 PM
@rkeazor by far the best way is to make testing easier is to leverage a Jetpack
ViewModel
to construct your own non-jetpack
ViewModel
(without
onCleared()
) with a
CoroutineScope
that is cancelled in the jetpack’s
onCleared()
. Injecting a
CoroutineScope
means you have no need to leverage
onCleared()
because
CoroutineScope
provides its own method for completion callback via
CompletionHandler
. There’s no better, cleaner way to do it.
r

rkeazor

04/14/2020, 7:06 PM
@zak.taccardi umm I don't know that sounds pretty strange to me. It just sounds like a extra layer of abstraction that isn't useful. The current implementation of ViewModel is pretty easy to test. You basically test it like any other class. You only need to leverage onCleared when your testing onCleared
z

zak.taccardi

04/14/2020, 7:07 PM
you should be shutting down any coroutines that are running from your test when your test ends
r

rkeazor

04/14/2020, 7:08 PM
yea but you don't have to do that manually
z

zak.taccardi

04/14/2020, 7:08 PM
correct. it is automatically done for you when you inject the
CoroutineScope
from
runBlocking { .. }
r

rkeazor

04/14/2020, 7:09 PM
yea but you dont need to inject coroutine scope to achieve that
for instance if I write a function
Copy code
fun CoroutineScope.addNumbers(...)
z

zak.taccardi

04/14/2020, 7:10 PM
correct. you can achieve that with a bunch of crazy, unnecessary `@Rule`s that you apply to your tests
r

rkeazor

04/14/2020, 7:10 PM
when I run this using runBlocking the scope is inherited from runBlocking
z

zak.taccardi

04/14/2020, 7:10 PM
we’re talking about unit testing ViewModels
r

rkeazor

04/14/2020, 7:10 PM
same as this
Copy code
suspend fun addNumnbers() { coroutineScope {
same
thing
z

zak.taccardi

04/14/2020, 7:11 PM
.viewModelScope
ignores the
runBlocking
you are wrapping your tests with because you violate structured concurrency
r

rkeazor

04/14/2020, 7:11 PM
you only add the rules when your creating a job within a function
z

zak.taccardi

04/14/2020, 7:12 PM
generally speaking if you are using coroutines, the only reason to directly access a
Job
instance is when you are doing something very complex
````
r

rkeazor

04/14/2020, 7:14 PM
So its not that viewModelScope in particular ignores runBlocking, is that there are times when you job is created outside the scope of runBlocking. This issue would happen regardless of viewModelScope
let me show you an example
👍 1
Copy code
fun addSomething(x: Int, y: Int) { 
launch { x + y  }
.....
}
this test will always fail if you use runBlocking
because you would need reference to the scope that is the parent of that coroutine
z

zak.taccardi

04/14/2020, 7:16 PM
correct, which is why you should make the parent injectable
r

rkeazor

04/14/2020, 7:16 PM
this is when you would need to set the main dispatcher
z

zak.taccardi

04/14/2020, 7:16 PM
I don’t use the main dispatcher for my view models
(though that’s an implementation detail)
I use
Dispatchers.Default
in production and the blocking dispatcher from
runBlocking { .. }
when testing (or
runBlockingTest(..)
)
My ViewModels don’t care because they are abstracted from it via the
CoroutineScope
injection
Effective Java Item 1 is to favor composition over inheritance, and that is what injecting a
CoroutineScope
into a class over extending Jetpack’s
ViewModel
achieves
r

rkeazor

04/14/2020, 7:20 PM
but see that issue isn't a issue with viewmodel, its just a logical complexity coroutine scope, but you can always use the coroutine scope builder and you will never see that issue
but in the end your still doing some level of inheritance, because your utilizing viewModel.
its just a extra layer of abstraction
if viewModel is already testable , I would argue that its easier to add a rule than boilerplate abstraction ,
a rule is like one line
z

zak.taccardi

04/14/2020, 7:24 PM
My view models are complex pieces of code. I reduce that complexity by injecting
CoroutineScope
. That
CoroutineScope
is then provided via the actual Jetpack
ViewModel
, which is abstracted via a single function call. This function is a standalone module that uses the Jetpack
ViewModel
as an implementation dependency so the rest of the codebase is not coupled to the Jetpack `ViewModel
a rule is like one line
One line per test. And if you forget to use it, test fails. Also breaks the ability to execute tests in parallel because you now rely on shared static state
r

rkeazor

04/14/2020, 7:26 PM
so what life-cycle do you tie your coroutine scope to ?
yea but testing rules aren't a bad thing. Its the devs responibility that they don't forget that lie.... But abstracting a ViewModel, to another ViewModel poses more trouble
z

zak.taccardi

04/14/2020, 7:28 PM
it depends on where my non-jetpack
MyViewModel
is scoped to. I usually scope to user flows rather than UI components like Fragments because then I can’t share that
MyViewModel
with other fragments. But this is abstracted by how the
MyViewModel
is provided
r

rkeazor

04/14/2020, 7:29 PM
First people joining your team won't know whats going on, because that architecture isn't synthetic to any architecture they would be accustom to. Two the Amount of BoilerPlate vs minmum gain , just doesn't match up
z

zak.taccardi

04/14/2020, 7:29 PM
there’s no boilerplate
not sure what you are talking about
r

rkeazor

04/14/2020, 7:30 PM
to create a viewModel , from jetpack viewmodel, there has to be some type of boilerplate
your writing more code somewhere
and that extract code is boilerplate. even if you extract it out to base class and use generics
z

zak.taccardi

04/14/2020, 7:32 PM
it’s the same as the existing jetpack view model boilerplate, but instead of a factory function that has a single parameter of
Class<T>
, it’s
Class<T>
+
CoroutineScope
r

rkeazor

04/14/2020, 7:33 PM
and all this is being done because to avoid a one line test rule
z

zak.taccardi

04/14/2020, 7:34 PM
one line in 50 test classes is 50 lines
r

rkeazor

04/14/2020, 7:34 PM
lol make a base test lol
it is still just one line
z

zak.taccardi

04/14/2020, 7:35 PM
why do we need to run code we don’t use?
we don’t use
Dispatchers.setMain(..)
anywhere because we inject our dispatchers
because it’s a best practice
r

rkeazor

04/14/2020, 7:36 PM
Its simple , the main dispatcher is tied to the Android Framework. During Unit test that dispatcher is none existent therefore, we should set it to the test specific dispatcher
this explanation is documented in the kotlin coroutines docs. which means people who code behind you only need to look at docs to know what going on
z

zak.taccardi

04/14/2020, 7:38 PM
We choose not to use
Dispatchers.Main
for performance reasons for our
ViewModel
s. So this would mean that every
viewModelScope.launch { }
would be incorrect because it would need to be
viewModelScope.launch(Dispatchers.Default) { .. }
every time. and to guarantee consistency there, that would mean to write a lint rule that would be unnecessary
r

rkeazor

04/14/2020, 7:38 PM
How much of a true performance gain do you believe you get from that? have you done benchmarks?
z

zak.taccardi

04/14/2020, 7:39 PM
referring to
dispatchers.main
vs
Dispatchers.Main
is straight forward
Why run code on the main thread that does not need to be run on the main thread?
r

rkeazor

04/14/2020, 7:43 PM
if your interacting with UI components which most times you are than you need dispatchers.main. if not you can always switch dispatchers
z

zak.taccardi

04/14/2020, 7:44 PM
the `ViewModel`’s
State
, exposed as
Flow<State>
, does not require the UI thread to build it fully
r

rkeazor

04/14/2020, 7:46 PM
I mean i see what your trying to accomplish dont get me wrong. But I think there isn't much benefit espeically compared to the cost. You can switch dispatchers whenever you want
its the same concept in RXJava
ObserveOn(AndroidSchdulers.MAIN)
z

zak.taccardi

04/14/2020, 7:48 PM
the only cost is that instead of using the Jetpack ViewModel factory, you use a different one with effectively the same API, just an additional parameter
Yes, in RxJava you should never directly refer to
AndroidSchedulers.MAIN
outside of the place you construct your dispatchers
r

rkeazor

04/14/2020, 7:51 PM
so are you saying you never create a class that extends ViewModel?
z

zak.taccardi

04/14/2020, 7:51 PM
I create 1
Copy code
internal class ViewModelHolder<T>(
    private val coroutineScope: CoroutineScope,
    val viewModel: T
) : AndroidXViewModel() {

    override fun onCleared() {
        coroutineScope.cancel()
    }
}
r

rkeazor

04/14/2020, 7:51 PM
yep boilerplate lol
z

zak.taccardi

04/14/2020, 7:52 PM
?
no
r

rkeazor

04/14/2020, 7:52 PM
def
and your cancling the scope. what your doing under the hood, is what viewModelScope does
its litterally a extract abstraction over whats already there
z

zak.taccardi

04/14/2020, 7:54 PM
correct, but it allows you to inject your dependencies
it’s also completely abstracted from the rest of the codebase
so it was a one time cost in a large codebase to fix a poor API provided by AndroidX, and has greatly reduced test complexity for the team
r

rkeazor

04/14/2020, 7:59 PM
I wouldn't say the API was poor. if it was poor than why make it the base lol. The thing is viewModel and viewModelScope do the exact same thing already
z

zak.taccardi

04/14/2020, 8:00 PM
by making assumptions and relying on static state to achieve it
r

rkeazor

04/14/2020, 8:00 PM
lol
its the same thing, trust me. your tieing the CoroutineScope lifecycle to the same static lifecycle that you hate
the benefit is trival at best
atleast in my opinion
i cant see the benefit
Both ViewModels are already testable and ViewModelScope, is just a CoroutineScope under the hood
z

zak.taccardi

04/14/2020, 8:03 PM
ViewModel
is nothing but a scoping mechanism. You can take that scoping mechanism and add additional responsibility to it, like building states for a view to consume. Of course, while minor, this violates single responsibility
I need that scoping mechanism of course
Both ViewModels are already testable and ViewModelScope
Easy to test and difficult to test are two different things
i cant see the benefit
You want to have some `ViewModel`s do background work in
Dispatchers.Default
. While others have a requirement to use
Dispatchers.Main
. How do you achieve that with
.viewModelScope
in a non-error prone way?
r

rkeazor

04/14/2020, 8:09 PM
one sec Zak, brb
👍 1
@zak.taccardi my bad , got caught up in some other things . ViewModel is pretty much a service locator. The complexity is only at creation time , and thats only in the perspective of a activity or fragment. So if you said it adds complexity to dagger, I would 100% agree with you . Creation can be complicated, even tho dagger has been improved to handle the situtation. But as for testing, you test it the same way you would test any class
Viewmodelscope, is just a coroutineScope that is already created for you and cleared for you in Viewmodel.onClear.... There is no more complexity pass that. So the complexity isn't in testing viewModelScope, or Google implementation of it. Its just the complexity of testing coroutines in general . which takes abit of understanding kotlin coroutines to get it right
Constructor injection is a viable solution for handling Coroutine-scope, after all there is no easier way to substitute a dependency during testing. So if ts thats your approach by all means. But there are also other ways wich arent complicated as well. as for abstracting out jetpack viewModel to be a none jetpack viewmodel...i thing that part still sound a bit overkill
z

zak.taccardi

04/15/2020, 4:59 PM
Viewmodelscope, is just a coroutineScope that is already created for you and cleared for you in Viewmodel.onClear.... There is no more complexity pass that.
this is becomes very complex if you need to: 1. change which dispatcher is used - in prod:
Dispatchers.Main
vs
Dispatchers.Default
- in test:
TestCoroutineDispatcher
2. Cancellation semantics: - in prod:
ViewModel.onCleared(..)
- in test: when
@Test fun testFunction()
completes
I’m specifically pushing complexity in creation time, using a very similar API to what AndroidX already provides. So to other devs, there’s no real additional complexity. Only complexity is the code I had to write to provide this improved, more flexible API, which was a one time cost
417 Views