-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Description
coroutines-guide-ui.md
uses GlobalScope for scope in it's examples, e.g. in section
At most one current job:
fun View.onClick(action: suspend (View) -> Unit) {
// launch one actor
val eventActor = GlobalScope.actor<View>(Dispatchers.Main) {
for (event in channel) action(event)
}
// install a listener to activate this actor
setOnClickListener {
eventActor.offer(it)
}
}
Unless I missed something, this solution looks like it might pollute GlobalScope
with actor jobs, as they're not tied to any finite view-related scope (i.e. multiple rotations of an Android device result in multiple click handler actors dangling). As I see it, ideally, some kind of local fragmentScope
/activityScope
should be used, for example lifecycle.coroutineScope
provided by LifecycleScope (of course, that would involve referencing the android-specific androidx.lifecycle:lifecycle-runtime-ktx
lib in the guide, which may or may not be in line with the purpose of the guide). Or probably the issue of managing the jobs may be worth mentioning as a side note next to the code.
I'll gladly help amending the guide via PR if the issue is, in fact, valid, and I apologise in advance if it's not and I just missed some piece of the puzzle.
Activity
elizarov commentedon Sep 3, 2019
@ivanbartsov Thanks. This is indeed a problem with
coroutines-guide-ui.md
, but it is not the only problem. This whole section needs to be rewritten withFlow
and structured concurrency in mind, but we'll undertake it only after the ui-related features ofFlow
are fully ironed out. However, if you have some reasonably sized patch that fixes the issues ofGlobalScope
without changing the whole guide structure, then we'll accept PR.