I'm doing migrate to coroutines refactoring in android application. Can someone help me with reviewing next implementation, please? To see if I has any obvious and nonobvious issues. The current idea (I don't want to rewrite whole app and every activity in one MR) is that it will allow to run multiple network requests in parallel between activity onCreate/onDestroy. Results are delivered through callbacks, yes. I know that this is not idiomatic, but I have to start conversion somewhere 😉

open class RetrofitCallManager( // Open because its mocked at some unit tests. 
    val uiDispatcher: CoroutineDispatcher = Dispatchers.Main, // To be able to switch to Unconfined at jvm tests.
    val ioDispatcher: CoroutineDispatcher = <http://Dispatchers.IO|Dispatchers.IO>
) : CoroutineScope {

    private lateinit var parentJob: Job

    override val coroutineContext: CoroutineContext
        get() = parentJob + uiDispatcher

    fun onCreate() {
        parentJob = SupervisorJob() // Job is supervised so one failed child coroutine wouldn't cancel other running coroutines.
    }

    open fun <T> execute(request: NetworkRequest<Result<T>>, listener: ResultListener<T>) = launch { // Launch returns Job which could be joined inside test runBlocking.
        try {
            val result = withContext(ioDispatcher) {
                request.loadDataFromNetwork() // Blocking retrofit network call.
            }
            if (result.isSuccess) {
                listener.onResult(result.data()!!) // Update ui.
            } else {
                listener.onError(result.error()!!) // Update ui.
            }
        } catch (e: CancellationException) {
            throw e // No need to propagate to UI since execution was cancelled by UI.
        } catch (e: Exception) {
            listener.onError(LocalError.create(ERR_CODE, e)) // Update ui.
        }
    }

    fun onDestroy() {
        parentJob.cancel()
    }
}

Is this something you would expect to see in android app?

I think it doesn’t make a lot of sense to use callbacks and join for launch

Thank you for checking! Just to make things clear for me which retrofit async API are you mentioning? Afaik, okhttp is still using blocking io. You mean just to delegate managing of thread pools to okhttp itself by using

call.enqueue(Callback

through coroutines-retrofit adapter from Jake?

Yes, use coroutines-retrofit adapter

Nice 👍 Haven't seen MR for the retrofit before.

Even not sure that you need this class in general

I don’t know details of your architecture of course, but I think it doesn’t help with migration and instead make sense to implement coroutine scope on your UI component where you already have onCreate/onDestroy or LifecycleProvider, so no need to write all this boilerplate

Actually I was wondering about this. I'm just starting with coroutines but if I learned anything in 8 years of doing android development its is that putting everything into activities is really bad idea. Leave ui for the ui and extract everything else. Why UI component should control/know what execution mechanism app is using? Is it really considered to be a best practice?

Okay, let’s split this to a few separate problems

I agree about moving to coroutines directly. Wasn't explicit enough with my disclaimer that that's the plan and current impl is a step#1 which allows me to use coroutines without changing any of 30+ screens or up to 50 network calls (not that I'm not using retrofit call directly but through my "requests" classes which are command pattern with some post process logic). That is the beauty of not coupling executing mechanism with UI/activities previously. Now I can switch it with one class only. (yes it's not fully performant of course, like cancellation is not propagated to okhttp, but it was the case with previous impl as well 😉)

I just think this migration is just bad idea, I would use old implementation for existing screens