Presenter with CoroutineScope

[mslalith]

Generally in many cases Presenter would require a CoroutineScope to do background work. At the time of this writing, we can create it using rememberCoroutineScope inside present() method. As this is local variable, we need to pass it down to functions which require them.

Suggestion to introduce CoroutinePresenter which contains coroutineScope as a immutable property within. This would be similar to above approach but would be created by Circuit and can be available as a class level property.

interface CoroutinePresenter<UiState : CircuitUiState> : Presenter<UiState> {
    val coroutineScope: CoroutineScope
}

I would like to initiate a discussion and understand the feasibility of this idea

[valeriyo]

Generally in many cases Presenter would require a CoroutineScope to do background work. At the time of this writing, we can create it using rememberCoroutineScope inside present() method. As this is local variable, we need to pass it down to functions which require them.

You shouldn't need to pass CoroutineContext into functions, really. Those functions should be suspend-able, and hence, they will have an implicit coroutine context, which they can use (in a "structural concurrency" fashion):

  suspend fun test() {
    coroutineScope {
      launch { delay(100) }
      async { delay(200) }.await()
    }
  }

Also, note that the coroutine scope returned by rememberCoroutineScope() is tied to the composition (and ultimately to the UI), so its not suitable for tasks that you wouldn't want to be cancelled because user hit Back.

But, how is this situation different from any other "thing" that a presenter needs to do its job? Use proper dependency injection (e.g. Koin) and supply a CoroutineScope instance (which you actually control the lifetime of) from the outside of your presenter:

internal class MyPresenter(
  private val coroutineScope: CoroutineScope,
  private val navigator: Navigator,
  private val thing: Thing,
) : Presenter<State> {
  @Composable
  override fun present(): State {
    // TODO
  }
}

[ZacSweers]

We almost never do raw IO operations in presenters. They are handled with data layers that live at a longer lived scope than presenters.

Cannot stress enough that Circuit wants you to use a good DI system + good data layer and just make presenters interpreters of that. If you start trying to do things like raw networking or otherwise-long-running tasks directly in your presenters and beholden to their lifecycles, you will have a bad time.

[mslalith]

@valeriyo I agree. CoroutineScope should not be passed down to lower layers. This can cause leaks or unintended behaviour.

As this is local variable, we need to pass it down to functions which require them.

I mean below. Let's say we have many events which needs to invoke suspending functions from Domain/Data layer. To call these methods we need a scope (assume these are tasks are tied to the screen). Since the scope is local to present() function, we need to pass to another function within the Presenter which would call suspending methods from Domain/Data layer.

This can be avoided if the scope is available as a class property.

If the scope needs to outlive the screen, then it needs to be injected. But in cases where the job is tied to the screen, then it makes sense to create a scope.

Chris Banes is experimenting around this in his tivi project: chrisbanes/tivi#1918

class ScreenAPresenter : Presenter<ScreenAUiState> {
    @Composable
    fun present(): ScreenAUiState {
        val scope = rememberCoroutineScope()

        return ScreenAUiState(
            ....
        ) { event ->
            when (event) {
                EventA -> doA(scope, ...)
                EventB -> doB(scope, ...)
                EventC -> doC(scope, ...)
                ...
            }
        }
    }

    private fun doA(scope: CoroutineScope, ...) {
        scope.launch {
            withContext(appDispatchers.io) {
                ....
            }
        }
    }

    private fun doB(scope: CoroutineScope, ...) {
        scope.launch {
            withContext(appDispatchers.io) {
                ....
            }
        }
    }

    private fun doC(scope: CoroutineScope, ...) {
        scope.launch {
            withContext(appDispatchers.io) {
                ....
            }
        }
    }
}

[valeriyo]

Passing scope into doX functions looks wrong. These methods should be all suspend-able. The launch-ing should be done within the present() method:

      EventA -> scope.launch { doA(...) }
      ... 

    private suspend fun doA(...) =
      withContext(appDispatchers.io) {
         ...
      }
 

This, beside being idiomatic, allows you to receive the return value of doA and handle it in the present() method.

As far as the CoroutineScope instance, you can use rememberCoroutineScope() or a scope that's passed from outside (i.e. injected):

class ScreenAPresenter(
   private val scope: CoroutineScope,
): Presenter<ScreenAUiState> {
   ...
}

Good luck!