badoo / MVIKotlin

Extendable MVI framework for Kotlin Multiplatform with powerful debugging tools (logging and time travel), inspired by Badoo MVICore library
https://arkivanov.github.io/MVIKotlin
Apache License 2.0
826 stars 66 forks source link

Would be nice to have a way to dispatch multiple Results in one shot #206

Closed dalewking closed 3 years ago

dalewking commented 3 years ago

Imagine that an Executor needs to make multiple updates to the state which requires dispatching multiple Results but you may want that set of actions to be applied as one atomic update to the State. Sure I could combine the notion into a single Result but that can lead to code duplication in the Reducer.

What would have been nice is if onDispatch (and dispatch in SuspendExecutor) instead of taking a Result instead took a vararg list of Results which were applied in one shot. Don't think you can change it now but perhaps could overload them to have a vararg version.

If you are wondering how applying a set of changes would look it would be like this:

newState = results.fold(state) { state, result -> state.reducer(change) }
arkivanov commented 3 years ago

Thanks! Are there any issues with producing multiple Results with multiple State emissions? I usually produce multiple Results when needed and never experienced any performance/UI/UX issues. This should work fine because there should be no thread switching between State emissions and UI updates, so the UI is always updated on the same call stack. Given you are using diff or any other similar thing (e.g. Jetpack Compose has this under the hood), this usually should not cause any problems.

But thanks again, I will think about how it could be implemented, will provide updates here.

Meantime, would the following workaround work for you?

class MyStoreFactory {

    private sealed class Result {
        data class First(val s: String) : Result()
        data class Second(val s: String) : Result()
        data class Third(val s: String) : Result()
        data class Combined(val results: List<Result>) : Result()
    }

    private object ReducerImpl : Reducer<State, Result> {
        override fun State.reduce(result: Result): State =
            when (result) {
                is Result.First -> copy(text = text + result.s)
                is Result.Second -> copy(text = text + result.s)
                is Result.Third -> copy(text = text + result.s)
                is Result.Combined -> result.results.fold(this) { state, inner -> state.reduce(inner) }
            }
    }
}
dalewking commented 3 years ago

It is more of a question of state consistency and the possibility of outputting a state that is not consistent. For example you might have an isValid property on the state that is a combination of other states. There are ways around it.

I am refactoring an existing view model and moving it toward an MVI API without actually adding MviKotlin so was implementing a dispatch and reducer myself and realized that it would be handy to have.

arkivanov commented 3 years ago

Thanks for the use case! I think that Reducer should be the only guarantee of the state consistency, as it is its primary responsibility. Relying on the fact that a series of Results should produce consistent state is very error prone. I recommend to actually create a separate Result class if the consistency is concerned.

dalewking commented 3 years ago

It just dawned on me that all that can be handled by creating a combined result type and handling it in the reducer, which is exactly the workaround you gave me that I glossed over. Closing as that is the correct way to do it rather than cluttering the dispatch function.