https://kotlinlang.org logo
Join Slack
Powered by
Why is `viewModelScope` being pushed by the Androi...
# coroutines
z
Why is 
viewModelScope
being pushed by the Android team? It requires a bunch of workarounds that would be completely unnecessary if the 
CoroutineScope
was just injected into the 
ViewModel
CoroutineScope
was designed so there’s no need to inject dispatchers either, so this type of nasty 
scope.launch(dispatcher)
is not needed
cc @Manuel Vivo
Watch the talk

https://youtu.be/KMb0Fs8rCRs▾

p
All companies push their products, you decide the adoption. Android is a huge ecosystem you own your code.
d
ViewModels have different lifecycle and lifespan then their users (Activities/Fragments). - Which CoroutineScope would you inject? - How would you manage difference in the lifecycles? - How would you manage canceling jobs in viewmodel when it's destroyed? lifecycleScope, whenCreated, whenStarted, whenResumed, and viewModelScope are just Android specific helpers which provide you with coroutine scope and are lifecycle aware to their respective owner to gracefully cancel jobs when parents become void.
➕ 3
z
Correct. But 
viewModelScope
seems to be an anti-pattern
You could use a special ViewModel Factory that provides a 
CoroutineScope
that is cancelled appropriately
d
sure you can, and hopefully you already do... But what you're proposing is that you implement your own CoroutineScope which you'll manually create, manage and cancel on ViewModel and all of that will basically make you write boilerplate code for each ViewModel only to name 
viewModelScope
to something different.
d
How would you change viewModelScope to fix which anti-patterns?
z
Why would we manually manage it? I'm advocating that a solution should be provided where it's managed for us
d
Do you just want it to be called viewModelCoroutineScope?
👎 1
d
"Why would we manually manage it?" Well if you DI it into ViewModel, what happens when ViewModel is destroyed and you don't cancel it manually? "I'm advocating that a solution should be provided where it's managed for us" Isn't that what viewModelScope literally is?
z
No. The correct solution is to allow constructor injection of scopes
Copy code
class MyViewModel(scope: CoroutineScope)
The reason 
viewModelScope
exists so you can get a scope that is associated with a 
ViewModel
. But what if it was provided to you from a 
CoroutineViewModelFactory
or something? instead of:
Copy code
create(modelClass: Class<T>): ViewModel
you would have:
Copy code
create(modelClass: Class<T>, scope: CoroutineScope): ViewModel
d
Again, whose responsibility is then to cancel the coroutineScope passed through constructor? How would framework know how to manage it correctly? How would you get: "I'm advocating that a solution should be provided where it's managed for us"
z
the library’s responsibility
the 
scope: CoroutineScope
would be cancelled for you, the same way 
onCleared()
is called for you
d
I don't understand that logic. Why would library have responsibility for handling object you created? What if you've passed helper coroutineScope which shouldn't be canceled at the end of ViewModels lifecycle?
z
d
I'm well aware of ViewModelProfider Factory and I use it...
z
you would not create the 
CoroutineScope
- it would be created for you
the same way you do not create the 
Class<T>
or call 
factory.create
yourself
d
Sorry, I don't understand what you have a problem with. If you want framework to create coroutineScope for you and manage it, it already does it... with 
viewModelScope
. If you want to create coroutineScope and be able to pass it to ViewModel through constructor using DI or whatever, framework doesn't know what you intend to do with it or how long it should live. You can freely do it from the introduction of ViewModelProvider.Factory, and you do not need use viewModelScope... no one is forcing you to use viewModelScope the framework gives you. If you want ViewModel to accept coroutineScope by default through constructor, but create the coroutineScope for you and pass it using that constructor, and manage it, then I don't follow you at all... let's wait for others to chime in...
👍 1
z
the framework doesn’t know what you intend to do with it or how long it should live.
Yes it does. The framework knows when to cancel the scope for you because it knows when to call 
viewModel.onCleared()
for you
Imagine that a proper 
CoroutineScope
was provided for you by the framework factory.
Copy code
override create(modelClass: Class<T>, scope: CoroutineScope): ViewModel {
  if (modelClass == MyViewModel::class.java){
    return MyViewModel(scope)
  }
  // ..
]
scope
above would be cancelled for you at the same time 
onCleared()
would currently be called
i
it is weird that onCleared() only called by factory
z
I'm not sure what's weird about that. View model lifetime is controlled by the UI component it is attached to
i
if you have single Activity with multiple fragments with view models, the view model will be cleared when users exited the activity.
👍 1
d
I wouldn't won't framework to kill scopes I created when ViewModel onCleared() is called, sorry. It can destroy the scope it has created for that particular ViewModel. It already does with 
viewModelScope
. That's fine... If I passed coroutineScope, I want to be responsible for it's own lifecycle. I created it, I own it, I'm responsible for it. Maybe particular ViewModel is used for logical portion of UI, say not even a screen's fragment but a nested fragment. If I passed scope which I'd like to reuse, I definitively don't want ViewModel to kill it on it's onCleared() Therefore my argument, framework cannot know when to cancel it. It's passed by constructor. ViewModel doesn't know if it was instantiated with ViewModelProvider.Factory, or manually, or in test or with DI, or... Only edge case classes should be responsible for objects passed to it. Otherwise they should stay clear... If I have Car and Engine objects, and instantiate the Car with Car(instanceOfEngine), I don't want resulting instance of car to kill/sell/do whatever with Engine instance. I'm responsible for it, not Car. What you're saying is Car knows exactly when to kill the Engine instance. I don't really see how it can know it...
z
The framework isn’t killing a scope you created. It would provide you with an already created 
CoroutineScope
that is scoped to the lifecycle of the 
ViewModel
Framework creates 
CoroutineScope
, not developer
If you watch the talk, there is a complex workaround to make unit testing of 
viewModelScope
possible. This complex workaround would be unnecessary if the scope from 
runBlockingTest { }
could be injected into the 
ViewModel
d
it already does... it's called 
viewModelScope
. What's with the constructor passing if the framework will create it anyway? It's like forcing framework's Car class to require Engine instance in it's constructor, but framework will always create this Engine instance for you. What's the point of passing by constructor then?
z
particular ViewModel is used for logical portion of UI, say not even a screen's fragment but a nested fragment. If I passed scope which I'd like to reuse, I definitively don't want ViewModel to kill it on it's onCleared()
If your 
ViewModel
outlives 
onCleared()
due to your UI being permanently destroyed , then you shouldn’t be using the `ViewModel`from Jetpack. You should define your own. In fact, this is what I do. Jetpack’s 
ViewModel
uses inheritance (bad), where if you just inject a 
CoroutineScope
into a custom 
interface ViewModel
(not the Jetpack ViewModel), then you have a lot more flexibility because the lifetime of the 
ViewModel
becomes an implementation detail of how you provide the ViewModel and the 
CoroutineScope
injected into it. I was requesting that a new 
CoroutineViewModelProvider.Factory
be created that would allow us to scope a 
ViewModel
to the UI (like 
ViewModelProvider.Factory
currently does), but instead of just providing a 
Class<T>
, also provide us with a 
CoroutineScope
that would be cancelled when the activity/fragment is permanently destroyed (when 
.onCleared()
) is called. We would never be calling 
scope.cancel()
ourselves, because that would be managed by the framework due us using 
CoroutineViewModelProvider.Factory
What's with the constructor passing if the framework will create it anyway?
because then you can easily swap out the framework provided 
CoroutineScope
when testing.
Copy code
runBlockingTest {
  val viewModel = MyViewModel(scope = this)
  // now test your view model
}
see how 
viewModelScope
complicates things

https://youtu.be/KMb0Fs8rCRs?t=534▾

d
I've seen all three sessions...
m
viewModelScope
reduces the boilerplate code of having to create and manage your own scope in the ViewModel. Not only reduces the boilerplate code but also reduces the chances of making a mistake (e.g. configuring it with a 
Job
instead of a 
SupervisorJob
). If you know what you're doing, you don't need it but there's a lot of people who find it useful. If you have other use case in which your coroutines need to run on a lifecycle different than the VM, then it's fine to get it injected (but this is a code smell, that code shouldn't be triggered by the VM). If not, you still need some sort of logic to get that scope cancelled when the VM is destroyed (which you get for free with vmScope)
👍 4
d
If your 
ViewModel
outlives 
onCleared()
due to your UI being permanently destroyed , then you shouldn’t be using the `ViewModel`from Jetpack. You should define your own. In fact, this is what I do.
What you missed here is my explanation that ViewModel and coroutineScope passed through constructor don't need to have the same lifetime. See

https://www.youtube.com/watch?v=B8ppnjGPAGE▾

at around 2:58 for a relevant slide. We have many ViewModels and hence related coroutineScopes. Again, nobody is forcing you to use 
viewModelScope
provided by library. You can create your own, pass it through the constructor, be it in app or in test, and use only that one. But for the n-th time, if you pass any object through constructor, it's your responsibility to end it. With tests it's easy, at the end of the test you can cancel it, but you can't do it from Activities and Fragments because they may have shorter lifespan then the ViewModel itself. What they described in the testing session explains why they use TestCoroutineDispatcher(), Dispatchers.setMain(), Dispatchers.resetMain() and how testDispatcher.cleanupTestCoroutine() weather entered manually or through JUnit4 Rule, catches leaked and/or unfinished coroutine scopes. You can manually do all that manually. I personally would rather not, and thus prefer frameworks 
viewModelScope
and testing Rule how it's proposed.
👍 1
👎 1
z
@Manuel Vivo `viewModelScope`is great for the quick and dirty, but a better solution could additionally be provided for those that want to facilitate easier testing
d
I'll leave @Manuel Vivo to continue... I don't see what I said differently then him. Framework provides you with 
viewModelScope
which is a convenience coroutineScope which is managed by library. You can provide your own, but then you need to manage it on your own... What you want you already can do anyway. It's just that you don't get library to hold your hand and cancel the passed coroutineScope for you.
z
But for the n-th time, if you pass any object through constructor, it's your responsibility to end it.
No, it's not. It's not your responsibility to call 
onCleared()
. Take the 
runBlocking
example below.
Copy code
runBlocking {
  val scope: CoroutineScope = this@runBlocking
}
scope
is provided to you as 
this
and you do not need to manage manually calling 
scope.cancel()
yourself When you use 
runBlocking { }
, it's not your responsibility to cancel the scope provided to you
m
It's a fair point. I'd rather say it's a solution that can be easy to understand and doesn't require a ton of work to get tests working (although I agree it adds some sort of boilerplate code to your tests - same as when testing LiveData, for example). I assume that having the possibility of being able to inject the Dispatchers that viewModelScope uses as default is feasible and I think it's been definitely considered in the past. Let me talk with Yigit and I'll get back to you :) cc @zak.taccardi
👍 2
d
It's not your responsibility to call 
onCleared()
!= You're responsible for objects you've created and passed through constructor
z
What you want you already can do anyway. It's just that you don't get library to hold your hand and cancel the passed coroutineScope for you.
Correct. I'm requesting that a library is provided to make achieving this as easy as possible, because it's the best solution. 
viewModelScope
is great when you aren't testing, but when you do need to test it has issues because you cannot constructor inject a different Coroutine scope easily
Wups, didn't mean to send that to the channel my bad
@Manuel Vivo thanks! Overriding the dispatcher used by 
viewModelScope
definitely makes sense. But I'm advocating that a separate 
CoroutineViewModelProvider.Factory
could exist that would provide the proper scope at instantiation time I'll create an issue to track this at some point in the future
d
ahm...
Copy code
class MyViewModel(val testingScope: CoroutineScope?): ViewModel() {

val myScope = testingScope ?: viewModelScope

...
in every ViewModel + ViewModelProvider.FactoryViewModelFactory vs
Copy code
@get:Rule val coroutineRule = MainCoroutineRule()
👍 2
m
That sounds good. By the way, out of curiosity, how would you compare in terms of amount of code to write and readability (considering junior developers as well) the boilerplate code of 1) just adding the JUnit rule to a test class vs 2) your approach? Edit: your current approach, not the desired API
👍 1
z
My current approach which is somewhat similar to the desired approach has equal boilerplate to using the regular factory for DI, though overall less boilerplate if you account for the fact that there's no test workaround weirdness
I get that you would still want to provide 
viewModelScope
for those that want to not leverage DI, but a special solution to inject a 
CoroutineScope
with a special factory would be awesome
s
For testing, you can still inject (using DI) a CoroutineContext, which may contain/be a TestCoroutineDispatcher....
👍 2
r
Some of these Ideas seem abit extreme. You shouldn't need a whole factory to create a viewmodel scope. Viewmodel already has enough factories lol. DI is always a good paradigm, but the scope of a Viewmodel should not be tied to the actual lifecycle of the viewmodel. You wouldn't want to inject the lifecycle scope for instance, because that would be a anti-pattern considering the fact that ViewModel is already lifecycle aware and was created to survive changes in the lifecycle .
The issue isn't really around viewmodelScope. ViewModel Scope , just handles the same boilerplate we type in ever ViewModel out the box
The actual issue is ,the complexity of testing Kotlin Coroutines.
Which for the basic cases , is pretty trivial , but for more complex cases, like dealing which channels and other complex co-routine can at times be abit tricky at times
s
But viewModelScope uses the Main Dispatcher, not available when running unit-tests. True, you can just inject the TestCoroutineDispatcher into the ViewModel and 'add' it to the already available viewModelScope. But I see nothing wrong with injecting the entire CoroutineScope into the ViewModel, when your code practices DI. The fact that the newer versions of the lifecycle libs now offer a ViewModel.viewModelScope property, doesn't change that. The scope (CoroutineScope) of the ViewModel is tied to its lifecycle. When onCleared is called, it's the end of its lifecycle and the scope will be cancelled. (Also using Dispatchers.setMain and such to 'inject' a fake/mock/test Dispatcher, that to me looks like an anti pattern; and what about the other Dispatchers like IO or Default?)
👍 1
r
But what CoroutineScope are you injecting? Where are you getting the scope your injecting from?
Its like I said the issues , you guys are talking about isn't about viewModelScope, its more about testing KotlinCoroutines. ..
Why wouldn't you use the main dispatcher? You can always switch to dispatchers.IO from viewModelScope
Why would Dispatchers.setMain a antipattern?
z
You want to use one dispatcher for production code (either main or default) and another for tests (
TestCoroutineDispatcher
)
Static state makes testing more difficult
☝️ 1
s
You're right, it's mostly about unit testing. My tests provide the right CoroutineScope or CoroutineContext/Dispatcher for running tests, my Activities provide the right ones for running on a real device. My ViewModels should not worry about that. When running test I want to run on the TestCoroutineDispatcher, for Activities on the Main Dispatcher.
z
Constructor injection makes testing easier
☝️ 1
s
And there aren't ones (setIO(...) or setDefault(...)) for the IO and Default Dispatchers,....
👍 1
z
Those are all functions to update static state to the correct state for tests
It's also about more than just the dispatcher, but the scope too. All the Coroutines need to complete before the test ends
👍 1
s
Yup, and I don't like that. (my response was for Robert)
👍 1
r
z
Also, unit tests should be trivial to run in parallel. If you need to share static state across unit tests, then tests can be longer be run in parallel
s
Robert, these docs indeed mention the use of the setMain(...) function, but advocate for injection for the other Dispatchers (I was at their talk at Android Dev Summit yesterday). That is not right, in my opinion.
r
This provides helpful ways to test Kotlin Coroutines. The first question, what scope are you actually injecting? To inject a dependency you actually need the dependency .
The reason why they use setMain is because Main isn't a . dispatcher tied to a platform
z
No..
s
Yup, and your ViewModel should not be worried about that. Hence, inject it
👍 1
r
"Dispatchers.setMain will override the Main dispatcher in test situations. This is helpful when you want to execute a test in situations where the platform Main dispatcher is not available, or you wish to replace Dispatchers.Main with a testing dispatcher."
👍 1
z
.setMain(..)
is used to change the dispatcher to one that works for testing so it's not the 
main
dispatcher anymore
r
yes I know that
👍 1
but he asked why there wasn't for io
why there was no setIO
z
Ohhh my bad, gotcha
r
its because main is the dispatcher tied to the actual platform
👍 1
s
And there should be one if I want to execute my tests quickly and in deterministic order
👍 1
r
and there is
s
For most of my unit tests, I'd like to run them all on one Dispatcher (TestCoroutineDispatcher), to finish them quickly and deterministically. I'd like to be able to pause the Dispatcher or forward quickly in time when I'm testing timeouts and delays.
👍 1
r
Have you read you the testing documentation for kotlin coroutines
z
Just so you guys know, I've been injecting both my 
CoroutineScope
and dispatchers and haven't had any difficulty testing Coroutines associated with that
s
With injection I can do all of that, have tests run in parallel, not worry about forgetting to 'clear' my 'main' dispatcher for tests, etc. Yup, I read them (I helped write some of its code, actually)
r
But what is the actual scope your injecting
z
In production?
Or in test?
r
prod
s
MainScope()
r
what lifecycle is it tied to
z
The same as 
viewModel.onCleared()
r
what
s
For tests, the TestCoroutineScope from the runBlockingTest lambda
r
Yes , what how are you creating the scope your injecting
z
Lifecycle is when the view model is instantiated to when 
.onCleared()
. I also use the default dispatcher to parallelize work
s
MainScope()
r
When is MainScope canceled?
z
I actually don't use the jetpack view model for this reason. I use it to provide a scope for my actual view model
s
When the onCleared is called on the ViewModel in which it is injected
z
Jetpack's 
ViewModel
can be used as an object graph for dependencies associated with the associated activity or fragment's full lifetime across config changes
s
Copy code
class MyViewModel(private val scope: CoroutineScope) : ViewModel() { 
    ...
    override fun onCleared () { scope.cancel() }
}
z
The downside to the above is that it's error prone. You could forget to call cancel
And if you inject 
CoroutineScope
, there's actually no reason to use 
onCleared()
, because you can leverage 
scope.coroutineContext[Job]!!.invokeOnCompletion { }
So there becomes no need to extend Jetpack's 
ViewModel
s
True, it depends on what your testing 😀
👍 1
r
I would argue that would be a anti pattern. You should always cancel your coroutine somewhere. anything else is prone to memoryleaks anti pattern
s
Depends on your tests, but I agree with you on that one. Let the ViewModel, which does call scope.cancel(), properly cancel the scope and lifecycle.
No memory leaks, though, just leaky abstraction.
r
If your not using JetPacK viewmodel, than you maybe wasting resources if you just the scope of a activity or fragment
s
No, the ViewModelProviders require the baseclass to be a ViewModel, so that onCleared can be called.... I wish ViewModel were an interface instead...
👍 1
r
Viewmodel in a sense is a service locator. But using it as a object graph sounds like extra boiler plate... In production code probably wouldn't scale nicely.. Just more and more boilerplate
But thats opinionated of course.
s
CoroutineContext or CoroutineScope is easy to inject. When using Dagger, for example, just use 
@Inject @MainScope val scope: CoroutineScope
in your ViewModel constructor.
r
but you wouldn't use the same scope for each of the viewmodel. you would always be creating a new instance correct?
s
Yup, the provider would not be a singleton, but a factory instead; new scope for a new ViewModel.
And for unit testing my specific ViewModel, I'd skip Dagger's injection and just inject/provide it manually
👍 1
Wow, this thread has gotten long! I love these conversations 😀
👍 1
z
Injecting 
CoroutineScope
is very scalable (composition/delegation instead of inheritance)
Using view model as a graph is very scalable as its confined to that fragment or activity and doesn't bleed anywhere
r
Using Viewmodel as a object graph for a viewmodel class you create. Thats what i mean doesn
t scale
z
Scales great
Lol
But in scenarios where I do that and I'm only providing a single object, which is my real view model, the whole view model as a graph concept is hidden as an implemention detail of some function
r
umm I don't know it sounds like extra abstractions and boilerplate. Perhaps calling it your object graph is the wrong word
There is alot complexity that goes into object graphs
Dependency scopes for example
z
I don't use Dagger as manual DI is simple enough
Scoping is very easy in my opinion. You just make a scope
r
Some dependencies , are managed like singletons, some are per activtiy, some can be tied to other lifecycle
z
Yep
r
You can't manage all that in a viewmodel properly
You would need some type of Super hash map. and it probably wouldn't be DI,
Maybe in a small project sure, but in a large project. or in production, I don't see how a ViewModel could handle all that in a scalable manner. it wasn't built for that
z
It was built exactly for that
A scoping mechanism for a fragment or activity across config changes
r
of course it wasnt lol. Viewmodel you can consider it a service locator ins sense. but its not a DI framework
In a production codebase , do you know how deep dependency graphs can grow? Exponentially lol ... Thats why finding a DI framework that scales is hard
👍 1
yes across configuration changes. but there is more to DI than just that..
z
Anyways we are pretty off topic now lol
r
you can't replace kotlin with Dagger. if one is not using JSR, one is not doing DI lol. Just hashmaps and kotlin sugars lol
yea true
I got to run actually guys. to finalize I don't think anything is wrong with viewmodelScope. Google and Jetbrains work closely together , so what Google deems best practices ,is more than likely what jetbrains deems best practices lol
but if you like injecting scope, there is probably no problem as long as your handle the lifecycle properly
later guys, good debate
👍 1
j
there is a complex workaround to make unit testing ... possible
it's The Jetpack Way™
👻 1
😆 4
252 Views
Open in Slack
PreviousNext
Powered by