#52 code cleanUp

[victorvicari]

Instead of return directly the information from repo, added to this layer a coroutine with context. And then, the view model will receive all the data.

[michaelbukachi]

Hey. I've just reviewed the code. Seems okay, though you'll have to incorporate newer changes. You should make the pull request against the develop branch not the master branch. We do active development on the develop branch.

[wangerekaharun]

Can you also consider making the runCatching a top level function (in a separate file) and also do we need to add a withContext(Dispatchers.IO) in our repo since we already had dispatcher defined in our viewModelScope

[wangerekaharun]

@victorvicari the last question aboutwithContext(Dispatchers.IO)?

[victorvicari]

It's an optimization for disk and network off the main thread. That, I believe fits quite alright in those cases.

[wangerekaharun]

Can you consider moving the withContext(Dispatchers.IO) inside the runCatching function so that you only do it once for all the repos.

[michaelbukachi]

We can also gracefully handle the result using snippet similar to the following:

fun <T : Any> Result<T>.onResult(onSuccess: ((T) -> Unit)? = null, onFailure: ((String?) -> Unit)? = null) {
    when (this) {
        is Result.Success -> {
            onSuccess?.let {
                it(this.data)
            }
        }
        is Result.Error -> {
            onFailure?.let {
                it(this.exception)
            }
        }
    }
}

[victorvicari]

Are you sure about replace the Coroutine context place? Actually, the best practice pointed out is that every function should handle the coroutine context on it's own way, and not make it a default context implementation.

[michaelbukachi]

It won't be the default implementation but the most used. Since every repo deals with IO there's repetition of code. Also since the pattern for repos is generally the same it would be good to refactor.

[victorvicari]

While I was migrating to this solution presented, a question popped up. If the lambda is null, what exception should I raise? If there's a Result, I'll catched it, the same for error. But for the null, what should I do?

[wangerekaharun]

At what instances will the lambda be null? Do you have the use cases?

[michaelbukachi]

null just provides a default value. Sometimes you are just interested in the success value. With the lambda having a default value of null you won't always have to specify a lambda parameter if you don't need to.

[wangerekaharun]

@victorvicari can you reslove the conflict so that we can merge your changes