Best practise for replacing current coroutine call in viewmodels

Question

I have the following:

interface CartRepository {
  fun getCart(): Flow<CartState>
}

interface ProductRepository {
  fun getProductByEan(ean: String): Flow<Either<ServerError, Product?>>
}

class ScanViewModel(
  private val productRepository: ProductRepository,
  private val cartRepository: CartRepository
) :
  BaseViewModel<ScanUiState>(Initial) {
  fun fetchProduct(ean: String) = viewModelScope.launch {
    setState(Loading)

    productRepository
      .getProductByEan(ean)
      .combine(cartRepository.getCart(), combineToGridItem())
      .collect { result ->
        when (result) {
          is Either.Left -> {
            sendEvent(Error(R.string.error_barcode_product_not_found, null))
            setState(Initial)
          }
          is Either.Right -> {
            setState(ProductUpdated(result.right))
          }
        }
      }
    }
}

When a user scans a barcode fetchProduct is being called. Every time a new coroutine is being set up. And after a while, there are many running in the background and the combine is triggered when the cart state is updated on all of them, which can cause errors.

I want to cancel all old coroutines and only have the latest call running and update on cart change.

I know I can do the following by saving the job and canceling it before starting a new one. But is this really the way to go? Seems like I'm missing something.

var searchJob: Job? = null

private fun processImage(frame: Frame) {
  barcodeScanner.process(frame.toInputImage(this))
    .addOnSuccessListener { barcodes ->
      barcodes.firstOrNull()?.rawValue?.let { ean ->
        searchJob?.cancel()
        searchJob = viewModel.fetchProduct(ean)
      }
    }
    .addOnFailureListener {
      Timber.e(it)
      messageMaker.showError(
        binding.root,
        getString(R.string.unknown_error)
      )
    }
}

I could also have a MutableSharedFlow in my ViewModel to make sure the UI only react to the last product the user has been fetching:

  private val productFlow = MutableSharedFlow<Either<ServerError, Product?>>(replay = 1)

  init {
    viewModelScope.launch {
      productFlow.combine(
        mycroftRepository.getCart(),
        combineToGridItem()
      ).collect { result ->
        when (result) {
          is Either.Right -> {
            setState(ProductUpdated(result.right))
          }
          else -> {
            sendEvent(Error(R.string.error_barcode_product_not_found, null))
            setState(Initial)
          }
        }
      }
    }
  }

  fun fetchProduct(ean: String) = viewModelScope.launch {
    setState(Loading)

    repository.getProductByEan(ean).collect { result ->
      productFlow.emit(result)
    }
  }

What's considered best practice handling this scenario?

Storing and cancelling the previous Job is exactly how to handle this. Is there a downside to this strategy you're concerned about? — Tenfour04, Oct 20 '21 at 13:18
I would have quite a lot of places in the app where I would eventually have to do this and it seems like unnecessary boilerplate code. I wish there was some easy way to replace the existing calls to have at max one call at any given point. Also, I wonder what happens if I rotate the screen, then the state of the saved job in the fragment would reset and a new one would be started I think. — nilsi, Oct 20 '21 at 13:26
the `viewModelScope` uses `Dispatchers.Main` so you actually don't want to run IO coroutines on it. To mitigate the screen rotation problem I'd go with some kind of cache on the `Application` level. There is no application coroutine scope so you'd have to create one as described [here](https://stackoverflow.com/questions/61255807/coroutine-scope-on-application-class-android) — Jakub Licznerski, Oct 20 '21 at 13:36
actually I was wrong, the [docs](https://developer.android.com/topic/libraries/architecture/viewmodel) show a nice diagram, where we can see that viewModel is not cleared on rotation (even though `Activity:onDestroy` is called) — Jakub Licznerski, Oct 20 '21 at 14:18

score 3 · Accepted Answer · answered Oct 20 '21 at 13:42

I can't think of a simpler pattern for cancelling any previous Job when starting a new one.

If you're concerned about losing your stored job reference on screen rotation (you probably won't since Fragment instances are typically reused on rotation), you can move Job storage and cancellation into the ViewModel:

private var fetchProductJob: Job? = null

fun fetchProduct(ean: String) {
    fetchProductJob?.cancel()
    fetchProductJob = viewModelScope.launch {
        //...
    }
}

If you're repeatedly using this pattern, you could create a helper class like this. Not sure if there's a better way.

class SingleJobPipe(val scope: CoroutineScope) {
    private var job: Job? = null

    fun launch(
        context: CoroutineContext = EmptyCoroutineContext,
        start: CoroutineStart = CoroutineStart.DEFAULT,
        block: suspend CoroutineScope.() -> Unit
    ): Job = synchronized(this) {
        job?.cancel()
        scope.launch(context, start, block).also { job = it }
    }
}

// ...

private val fetchProductPipe = SingleJobPipe(viewModelScope)

fun fetchProduct(ean: String) = fetchProductPipe.launch {
        //...
    }

Super, I think I will go for something like that. Thank you! — nilsi, Oct 20 '21 at 13:48
@TenFour04 what about the context switch? Looks like the SingleJobPipe would be used to run some IO operations. Would you recommend to put `with(Context.IO)` inside the `SingleJobPipe:launch` ? — Jakub Licznerski, Oct 20 '21 at 14:20
I wouldn't it because it'd be overstepping its responsibility. It's a bridge to using an existing scope, so it should use that scope's default dispatcher. The convention on Android is for most coroutines to start on the Main dispatcher and only wrap the pieces of code that need a different dispatcher. And that's usually done at a lower level than a launched coroutine anyway. For instance, if you are using Room or Retrofit, they provide suspend functions so they already delegate to an appropriate dispatcher under the hood. OP's example uses a Flow so it doesn't need a specific dispatcher. — Tenfour04, Oct 20 '21 at 14:30

Best practise for replacing current coroutine call in viewmodels

1 Answers1