https://kotlinlang.org logo
Title
b

Bradleycorn

03/02/2021, 3:58 PM
Android specific question re: coroutines ... is there any issue with doing something like this?
class MyViewModel(): ViewModel() {
    private var job: Job = viewModelScope.launch {
        // Some long running async task, like polling a server or something
    }

    fun onSomeUIEvent() {
        job.cancel()
    }
}
n

Niklas Gürtler

03/02/2021, 4:01 PM
Why nullable?
b

Bradleycorn

03/02/2021, 4:03 PM
Whoops. doesn't need to be. at first I was putting the
launch
in an
init {}
and starting the job as null ... but then realized I can just set it straight away in the constructor.
n

Niklas Gürtler

03/02/2021, 4:03 PM
Right... Should be OK i think
f

fmasa

03/02/2021, 6:43 PM
I don't see a problem per se. But what exactly are you doing in that job? The only issue I can think of is not coroutine-specific but rather Android specifcic. With current approach this job will still run until it's either canceled or viewmodel is destroyed. This means that when your app is in background, this job may be still running (it's not lifecycle aware).
u

uli

03/02/2021, 7:07 PM
It's launched in viewModelScope @fmasa
f

fmasa

03/02/2021, 7:10 PM
@uli Yes, but that scope is closed when view model is cleared. Which happens when activity/fragment is destroyed, not necessarily when user switches to different app. This may or may not be what OP intended.
b

Bradleycorn

03/02/2021, 11:20 PM
@fmasa, agreed that the lifecycle of the view (Activity/Fragment) that the ViewModel is attached to must be taken into account.
👍 1
a

Adam Powell

03/03/2021, 1:14 AM
I tend to try pretty hard to avoid launching any sort of ongoing behavior as a side effect of a constructor. In the case of coroutines you can sometimes kind of squint at the idea of setting up an actor during construction that launches, performs no side effects, and then immediately suspends awaiting a command message of some sort and say that doesn't count.
b

Bradleycorn

03/03/2021, 1:29 AM
yeah, and my example was probably a bad one, as my question was more to do with holding a reference to the
job
in the ViewModel, more-so than launching from the constructor. I do see the risk in that. A better example for my question might be more like:
class MyViewModel(): ViewModel() {
    private var job: Job? = null

    fun onDownloadClick() {
        job = viewModelScope.launch {
            // Download some big file
        }
    }

    fun onUserCanceled() {
        job?.cancel()
    }
}
a

Adam Powell

03/03/2021, 1:56 AM
you're fine
👍 1
l

leandro

03/03/2021, 5:16 PM
Adam, to your comment of avoiding launching at construction time, I do this in a couple classes only, mostly because I haven’t found a better solution. Would you rather have e.g. a method
.init()
that would launch a coroutine?
a

Adam Powell

03/03/2021, 8:01 PM
it mostly depends on what the launched coroutine is doing. If it's doing some sort of poll or data refresh I tend to prefer things like cold flows. If some sort of sharing or completion of an operation across config changes is appropriate, I usually try to scope that to a more app-scoped repository instead of a viewmodel, as it's usually a good sign that the data I'm working with has a wider scope and lifetime than just the viewmodel anyway.
it's not that it can't be made to work, I just find it easier to reason about the system when I keep the number of entities that are capable of acting autonomously to a minimum
when I need something to be running I'll often add something like a
suspend fun run()
method to the object and call it from some other scope when it needs to be active, like lifecycle-bound scopes from a UI, etc.