Android LiveData and Coroutines - is this an anti-pattern?

Question

I've inherited a Kotlin Android project from a former developer. He used suspend functions to handle, for example, network requests. An example of such a function might be this:

suspend fun performNetworkCall(param1: Long, param2: String): ResultOfCall
{
   Do()
   The()
   Stuff()

   return theResult
}

So far, so good. Now he has ViewModels for his fragments and he also has methods in these models that should call the above suspend function asynchronously and return some results. He does it like this:

sealed class LiveDataResult<out R>
{
    data class Success<T>(val result: T) : LiveDataResult<T>()
    data class Failure(val errorMessage: String) : LiveDataResult<Nothing>()
}


fun fetchSomeData(param1: Long, param2: String): LiveData<LiveDataResult<String>> = liveData {
    val resultOfCall = performNetworkCall(param1, param2)
    if (resultOfCall indicates success)
        emit(LiveDataResult.Success("Yay!")
    else
        emit(LiveDataResult.Failure("Oh No!")
}

In his fragments he calls such methods like

viewModel.fetchSomeData(0, "Call Me").observe(viewLifecycleOwner) {
    when (it)
    {
        is LiveDataResult.Success -> DoSomethingWith(it.result)
        is LiveDataResult.Failure -> HandleThe(it.errorMessage)
    }
}

I'm not very experienced in the entire observable/coroutine matter yet, so my questions on this approach are:

1. Wouldn't that pile up a whole bunch of LiveData objects that won't get released due to the observer still being attached?
2. Is this a bad approach? Bad enough to refactor? And how should one refactor in case it should be refactored?

Answer 1

I'm not an expert on this subject, so I'm just speaking from my understanding of how coroutines and LiveData work based on browsing source code.

A typical LiveData is not kept alive by having observers. It's like any other typical object that's kept alive by being referenced.

CoroutineLiveData, however, once started will be kept alive by its coroutine continuation. I think the coroutine system maintains a strong reference to the object with the suspend function until the suspend function returns and the continuation can be dropped. So each instance of the LiveData created by the fetchSomeData function will run to completion, even if the observer has reached end of life. When the network call is complete, nothing is left to hold a reference to the LiveData, so it should be cleared from memory.

So, there is only a temporary leak happening. Your network calls won't be cancelled if the Fragment that made the request is closed before receiving the result. This is because CoroutineLiveData uses its own internal CoroutineScope that is not tied to any lifecycles. If you reopen the Fragment multiple times, such as by rotating the screen, you could have multiple obsolete network requests still running. There are apparently work-arounds to manually cancel, but it's kind of messy.

Also, in my opinion, using LiveData for fetching a single result is just inserting extra complexity and sacrificing automatic cancellation, when you could call a suspend function directly. Modern versions of libraries like Retrofit already have suspend functions for making requests, so network request cancellation would automatically happen if the suspend function is called on a CoroutineScope that is tied to a lifecycle.

A refactored version of this code that would support automatic cancellation might look like this:

suspend fun performNetworkCall(param1: Long, param2: String): ResultOfCall
{
   val result = setUpAndDoSomeRetrofitSuspendFunctionCall(param1, param2)
   return result
}

sealed class NetworkResult<out R>
{
    data class Success<T>(val result: T) : NetworkResult<T>()
    data class Failure(val errorMessage: String) : NetworkResult<Nothing>()
}

suspend fun fetchSomeData(param1: Long, param2: String): NetworkResult<String> 
{
    val resultOfCall = performNetworkCall(param1, param2)
    return if (resultOfCall indicates success)
        NetworkResult.Success("Yay!")
    else
        NetworkResult.Failure("Oh No!")
}

// In Fragment:
lifecycleScope.launchWhenStarted 
{
    when (val result = viewModel.fetchSomeData(0, "Call Me"))
    {
        is NetworkResult.Success -> DoSomethingWith(result.result)
        is NetworkResult.Failure -> HandleThe(result.errorMessage)
    }
}