A case against the MVI architecture pattern

The opinions in this article are based on my own experience working with different implementations of MVI in both personal projects and at my day job.

To begin with, let me say I recognise how appealing MVI looks at first sight. It combines battle tested patterns like Command and Observer and makes good use of functional reactive programming techniques. In short, MVI sort of defines a meta-architecture favouring classes instead of functions and where every action the user can take is defined by an Input class and every state a view can be in must also be defined by a subclass of the State class.

I'm assuming you already know how this works, there are more classes like these in MVI including Actions, Results; among others, depending on each specific implementation.

The problem with MVI

My biggest pet peeve with this architecture is NOT how boiler-platy it can get, or how opinionated certain implementations can be.

My biggest problem with this architecture is how much it hurts overall code readability and in turn, developer happiness.

In my opinion, traditional MVI implementations artificially group ‘unrelated code’ together and encourages ‘related code’ to be spread out among as many different classes as possible (low coupling, high cohesion)
That's a big claim, let me explain what I mean.

When writing a new piece of code, you only care about the feature you are writing/reading and not much else. If you are fixing a bug, you are usually focused on a specific flow where the bug is happening, trying as hard as you can to reproduce it. The faster you can build a mental model of how this feature works the faster you'll fix the bug. So it helps if the code for the feature grouped together and isolated from any other feature.

Unfortunately, this is usually how code ends up looking in MVI:

    fun intentToAction(intent: HomeIntent): HomeAction {
        return when (intent) {
            is HomeIntent.LoadAllCharacters -> HomeAction.AllCharacters
            is HomeIntent.ClearSearch -> HomeAction.AllCharacters
            is HomeIntent.SearchCharacter -> HomeAction.SearchCharacters(intent.name)
            is ...
        }
    }

See how all features are intertwined among other completely unrelated features in that snippet? You probably only care about one single line in that method at a time.

In other words, you are never, ever, going to read that snippet of code from top to bottom (you know, like we usually read things). So why are we writing it like that? It's hard to write, it's hard to read.

You might be thinking "well, that doesn't looks so bad". Now imagine 10 or 20 more lines in the when statement, which basically only perform a trivial mapping from two very similar sounding classes FooInput -> FooAction.

It doesn't stop there though, in many implementations actions get mapped to results, and then results get mapped to states:

fun Result<List<Persona>>.reduce(isSearchMode: Boolean = false): HomeState {
    return when (this) {
        is Result.Success -> if (isSearchMode) HomeState.ResultSearch(data) else HomeState.ResultAllPersona(data)
        is Result.Error -> HomeState.Exception(exception)
        is Result.Loading -> HomeState.Loading
    }
}

Take a moment to think how you would follow the code in your head while trying to find that nasty bug. You start from the View, jump to the ViewModel, and see the first mapper (inputs -> actions). From there there's not clear path to take (or IDE shortcut to use) to jump to where the flow continues (since the architecture usually hides the wiring from the clients). You basically need to do a cmd+f of the action name (or cmd+b to get a list of places where it's used/declared) to try and find where that action is being processed, that's usually in the VM as well, some implementations define a UseCase class and place the mapper there, it really could be anywhere.
Ok, you found the Actions -> Results mapper, you found the one line you care about in that mapper, what now? Well now you jump yet again! this time to the Reducer and you find... another mapper! At this point you forgot what you where doing entirely.

MVI makes reading code a very awkward experience. It’s analogous to reading a book where each next word is on a new page. You need to keep flipping pages (going to a different class) to know what comes next. You end up spending more time finding code, than reading code.

All this added overhead makes it easy for bugs to hide in plain sight. This gets more tricky when you take into consideration Threading, RxJava/Coroutines, Asynchronous events, Lifecycle scopes, etc.

I understand where MVI is coming from here though, if multiple Inputs produce the same Action then we can simplify things a bit. Unfortunately, in practice this is almost never the case. Your flows won't share Inputs, Actions, or Results`. It's more likely that for every "flow/usecase" in your app, you'll have one of each.

Reducer is a leaky abstraction

Don't get me wrong. There are valid reasons for extracting code into other classes.
A good example of this are ViewModels. A ViewModel exists not because a particular architecture requires it, it exists because it has a clear purpose. It serves as an interface from the view to the business logic, separates the rendering view code from non-rendering related code, it signals a larger scope (at least in android the lifecycle-scope of the VM is slightly larger than the scope of the views), facilitates testing, etc. In other words, it has a propose that's beyond the chosen architecture agenda.
In the same way, UseCase classes exist because UseCases are a reusable component that might be used from different VMs. Same goes for Repositories, a repository can stand on its own and serve multiple clients. All these classes exist with a clear goal, scope and purpose. Here, the logic is split sensibly.

On the other hand, let's look at MVI's Reducer. A Reducer is very specific to a particular VM (not reusable). Moreover, the scope of the reducer is the same as the scope of the VM. Why extracting it out out into its own class?
"For testing purposes" I hear you say. That's not a good enough of a reason IMO.
Ask yourself this: "Should I really be extracting private methods into a separate classes solely to be able to test them?" I personally don't think so, those methods are private for a reason, they are implementation details, they are not meant to be unit tested directly. Otherwise, what's stopping you from extracting every single private method from every single class in your project and testing those too?.

**It's perfectly normal to want to simplify your classes when the get too big and complex. But in my opinion, extracting things into different classes needs to be done in a sensible way.

What I'm trying to say here is, you can come up with all sort of ways of splitting code, but not all of them will be good.

Splitting your VMs based on functionality/use-case/features is usually better and makes more sense than splitting it by some arbitrary architecture convention. As an example, consider these two classes:
MediaCache: This class handles caching for images and video
Reducer: This class reduces results to states (huh?).

Here, the Reducer could potentially grow infinitely since it could host logic for any new feature you add to the View/VM. The purpose of the class is very abstract and tied to the architecture.
On the other hand, there are few operations that you can put into MediaCache, it has a well defined scope based on functionality (what the class does). Sure, you can get very fancy and design a very complex cache mechanism but all your code will be within scope of caching things. It's not the case with Reducers, the scope will grow and grow and as you add more and more new features.

Extracting implementation logic into trivial classes only for testing proposes also results into simplistic tests that become increasingly meaningless and add a ton of friction to your codebase. If you want to read more about why testing private methods is bad I highly recommend this read: Unit testing private methods by Vladimir Khorikov

When you have a class with a single function reduce and no state, you should ask yourself "why is this a class to begin with?". Just move the function inside your VM, no need to extract it into artificial classes. Then go a step further and get rid of this function altogether, because again, reducing should happen contextually within the feature and it should not be artificially grouped together in a function.

Ending thoughts

There are many other reasons I dislike this architecture, It's very intrusive, it adds a ton of boilerplate, it can be ambiguous, etc. Those are not exclusive to MVI and I could live with some of those drawbacks. I'm sure MVI might fit some applications better than other, but in all cases, readability suffers.

All in all, MVI looks elegant from a theoretical standpoint but I have yet to see an implementation that actually works and scales well for big projects.

I'm very happy that Jetpack compose is just around the corner! Up until now I've been using a hierarchy of custom views + view models in my apps (something that kind of resembles the composable approach that Google is taking with Jetpack Compose). It's nice to know that his approach will soon become more mainstream.

Website

Twitter

EDIT:

I've been told not all implementations extract the reducer into its own class. I think that's a good thing! Still, most MVI implementations unnecessarily force you to reduce ALL your features in the same function/class (resulting in giant when statements, mixing unrelated features, grouping unrelated code, and making things hard to read overall).

I was also told that big when statements/mappers can be prevented by using composable "sub-view-models". IMO, splitting this when statements into smaller ones does not fix the underlying problem, it only adds more boilerplate. More classes that just redirect you somewhere else. You are still reading code in a perpendicular way as to what you write (one line at a time before jumping somewhere else). And again, you end up spending more time trying to find code than actually reading it.

Lastly, I've been presented with this architecture: https://github.com/orbit-mvi/orbit-mvi
Which I haven't tried but I like what I see, it throws away many MVI made up restrictions while keeping the ones that make sense. Feature flows read linearly and the state reduction happens contextually (as opposed to grouped into one big "reducer" function/class).