Android specific question re: coroutines ... is there any issue with doing something like this? ```c...

# coroutines

[Bradleycorn]

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()
    }
}

[Niklas Gürtler]

Why nullable?

[Bradleycorn]

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.

[Niklas Gürtler]

Right... Should be OK i think

[fmasa]

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).

[uli]

It's launched in viewModelScope @fmasa

[fmasa]

@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.

[Bradleycorn]

@fmasa, agreed that the lifecycle of the view (Activity/Fragment) that the ViewModel is attached to must be taken into account.

[Adam Powell]

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.

[Bradleycorn]

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()
    }
}

[Adam Powell]

you're fine

[leandro]

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?

[Adam Powell]

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.

[Adam Powell]

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

[Adam Powell]

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.