Misc considerations for `SplashActivity`

[AllanWang]

Going along with good practices, if you have functions that should never run on the main thread, or functions that should always run from the main thread but aren't always called from a main thread, it would be better to make them suspend functions and wrap them in their expected context. This way, the caller doesn't have to worry about the coroutine context, and it will be much easier to maintain as the specifications occur only once.

For instance, showNextScreen modifies views but is sometimes called in the background, It would be nicer to make it suspended with a ui context, and use launch { showNextScreen() } in onActivityResult rather than withContext(uiDispatcher) { showNextScreen() } from the background caller.

In the case of login, you can see that it is specified to begin with the ui dispatcher, yet is called from launch(bgDispatcher) { login(true) } by showNextScreen. While this works, this is an incorrect context switch by the caller.

There are other parts, such as

// currently in bgDispatcher
if (downloadEverything) {
    try {
        userDownloader.execute()
    } catch (e: IOException) {
        // If there was an exception, stop here
        // Hide the container
        progressContainer.isVisible = false

        showLoginScreen(e)
        return@withContext
    }
} else {
    userDownloader.start()
}

withContext(uiDispatcher) {
    // Hide the container
    progressContainer.isVisible = false

    // Connection successful: home page
    openHomePage()
}

where the call within downloadEverything is not in the right context. As showLoginScreen assumes it starts in ui, I believe that will crash, as well as the progressContainer.isVisible = false before it

[AllanWang]

A few other things:

Result is not a part of coroutines. While you can't create one yourself, you can use runCatching to output one.

If you wish to keep using your own Result, then EmptySuccess can probably be an object rather than a class

[jguerinet]

I agree with all of this. I hate the current SplashActivity, it's very convoluted and does way too much in too little functions. The coroutines are a mess, they were just set up to work essentially, and are not optimized. I want to revisit this entirely, especially after refactoring the way data is downloaded.