Why does a normal function need to be wrapped with viewModelScope.launch?

Viewed 684

The following code is from the project.

1: In my mind,a suspend fun should be launched in another suspend fun or viewModelScope.launch{ }, withContext{ } ... , filterItems() is only a normal function, I don't know why filterItems() need to be wrapped with viewModelScope.launch{ } in the function filterTasks(), could you tell me ?

2: In the function filterTasks(), viewModelScope.launch{ } will launch in coroutines, it's asynchronous, I think return result maybe be launched before I get the result from viewModelScope.launch{}, so the result maybe null, is the code correct?

Code

   private fun filterTasks(tasksResult: Result<List<Task>>): LiveData<List<Task>> {       
        val result = MutableLiveData<List<Task>>()

        if (tasksResult is Success) {
            isDataLoadingError.value = false
            viewModelScope.launch {
                result.value = filterItems(tasksResult.data, getSavedFilterType())
                //return filterItems(tasksResult.data, getSavedFilterType())  //It will cause error.
            }
        } else {
            result.value = emptyList()
            showSnackbarMessage(R.string.loading_tasks_error)
            isDataLoadingError.value = true
        }

        return result //I think it maybe be launched before I get the result from viewModelScope.launch{}
    }


    private fun filterItems(tasks: List<Task>, filteringType: TasksFilterType): List<Task> {
        val tasksToShow = ArrayList<Task>()
        // We filter the tasks based on the requestType
        for (task in tasks) {
            when (filteringType) {
                ALL_TASKS -> tasksToShow.add(task)
                ACTIVE_TASKS -> if (task.isActive) {
                    tasksToShow.add(task)
                }
                COMPLETED_TASKS -> if (task.isCompleted) {
                    tasksToShow.add(task)
                }
            }
        }
        return tasksToShow
    }
3 Answers

It doesn't, unless it performs some heavy work and you want to move it to a background thread, which is the case here. Here the author just wanted to disjoint the work so the live data can be updated with an empty list first, and the filtered list later(computationally intensive to get), but forgot to do it out of the main thread.

In this particular case the author may have forgotten to add a background dispatcher as a parameter

viewModelScope.launch(Dispatchers.Default)

hence, in this scenario the intended behavior was not achieved, so you see this "nonsensical" coroutine.

I think you can contribute to the project with a fix :)

yes, you are right. but if you looked up the implementation of the launch {} such in lifecycleScope.launch {} or viewModelScope.launch {} you would find out the "block" which is "the coroutine code which will be invoked in the context of the provided scope" is cast to be suspend, so any block of code between launch {} is suspend code block. so in your example filterItems is cast to suspend under the hood and it's wrapped with viewModelScope.launch{ } to do its heavy task not in main thread.

public fun CoroutineScope.launch(
    context: CoroutineContext = EmptyCoroutineContext,
    start: CoroutineStart = CoroutineStart.DEFAULT,
    // the below line is doing the magic
    block: suspend CoroutineScope.() -> Unit
): Job {
    val newContext = newCoroutineContext(context)
    val coroutine = if (start.isLazy)
        LazyStandaloneCoroutine(newContext, block) else
        StandaloneCoroutine(newContext, active = true)
    coroutine.start(start, coroutine, block)
    return coroutine
}

I agree that the code looks suspicious, main reason is that it launches the filterItems coroutine into the Main dispatcher, basically just postponing the moment when filterItems will run on the GUI thread. If filterItems takes long to complete, it will block the GUI; if it doesn't take long, then why would you launch a concurrent coroutine in the first place?

Furthermore, on an architectural level, I don't see a reason why you'd have a function returning LiveData<List<Task>> when you can just have a suspend fun returning List<Task>.

Related