-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Common usage of CoroutineScope Job functions are difficult to understand #934
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
Comments
We have been thinking about this for a while. The problem with a lot of extensions of This issue is more about design of the mental model and clear understanding of |
We are using: job.cancel()
job.cancelAllChildren()
job.invokeOnCompletion()
job.isCompleted // rare
job.isActive // rare
job.cancelAndJoin() // rare
I'm currently using it to signal a
Ultimately, I think it's important to understand that the handle on coroutines created from a I don't think it's very important (for beginners) to understand that a val scope = CoroutineScope(Dispatchers.Main) // specify the dispatcher
// control lifecycle
scope.job.invokeOnCompletion { } The above is simple to onboard beginner devs to. The following is not: val scope = CoroutineScope(Dispatchers.Main)
scope.coroutineContext[Job]!!.invokeOnCompletion { } It requires a dev to understand that:
Not requiring a dev to understand those things at the very beginning for the very basics would be a big win |
Even if we ignore the mental model and discoverability issues, I am afraid that introducing E.g.
If we introduce We can workaround it with |
I'd use this opportunity to take a better look at
|
Note: I'm using
Sure. Consider how the Android class MyViewModel(private val scope: CoroutineScope) {
init {
scope.coroutineContext[Job]!!.invokeOnCompletion {
// clean up anything that would need to be cleaned up in response to `onCleared()`
}
} |
I actually don't use that function at all for the race condition reasons. Some other members of my team like it because it gives them cleanup at a low level of effort. They use it to cancel network requests that are running at a "global" scope. (think like a There's also this example |
What kind of cleanup you'd typically do in a As a side note, the original design was based on architecture in which
So my other question would be if you have to inject a scope into the view model and why. |
Because I use the injected
No. They just expose a
class MyViewModel(scope: CoroutineScope) : ViewModel, CoroutineScope by scope { .. } My
I would actually disagree - it's whoever calls |
One problem we have on our team when onboarding developers to Coroutines is the complex access to the
Job
in aCoroutineScope
- specificallyscope.coroutineContext[Job]!!
. Access to methods like.invokeOnCompletion
,cancel
, etc are unwieldy to use because of theJob
nullability and violation of the Law of Demeter.The latter is more difficult because you have to understand that a
CoroutineScope
is just a wrapper around aCoroutineContext
that will always have aJob
(even though the API says aJob
can benull
). And once it is undestood, it's a pretty verbose API.Possible Solutions
I see two possible solutions going forward (though both could be implemented).
.cancel()
,invokeOnCompletion{ }
, etc as extension functions onCoroutineScope
.scope.job
that returns an non-nullJob
- because aJob
should never be null for aCoroutineScope
.note:
scope.cancel()
was added in #866The text was updated successfully, but these errors were encountered: