<https://medium.com/androiddevelopers/viewmodel-on...

# orbit-mvi

[Ravi]

https://medium.com/androiddevelopers/viewmodel-one-off-event-antipatterns-16a1da869b95 any thoughts on this as I’m using SideEffect pattern from orbit-mvi

[Mikolaj Leszczynski]

Great to see Google providing a rationale for their new guidance finally! Nice article.

Here’s how this relates to Orbit:

Antipattern #1: State about payment completion can be lost

This is not an issue with Orbit, since we ensure delivery by only sending the events when there are subscribers listening, and caching them if there are none.

That said, this prevents being able to multicast the side effects (as with State). However in 99% of cases this is not a problem.

Antipattern #2: Telling the UI to take an action

Yeah fair points there. I have no counter arguments 🙂

Antipattern #3: Not handling the one-off event immediately

To be honest, I’ve read this several times and don’t get this point.

Something that is not super clearly communicated are the major downsides of this approach.

1. Every event must be handled by the UI. This means the UI needs to call a ViewModel method to signal that it has “handled” the event. This creates extra verbosity.
2. Repeatable one-off events are more difficult to model - your state would require some sort of list for these. If you used a field, they might get overwritten.
(2) should rarely happen, but (1) is important!

From our own experience, UI toolkit is poorly suited for handling these acknowledgements. I wouldn’t recommend using it with state-based one-off events.

However, if you use Jetpack Compose this becomes a viable strategy and I can certainly see the appeal! Compose is much better at handling frequent state updates and two-way binding situations.

This is just my personal opinion of course! With Orbit, you are free to use either approach!

I’m curious to hear what others think.

[Mikolaj Leszczynski]

Antipattern #3: Not handling the one-off event immediately

OK, I think I get this one.

Imagine you get an event with a one-off event inside. When handling the event, you need to immediately “handle” the event by calling the ViewModel to change the state to reflect that.

If you do that in a multithreaded environment you run the risk of repeating the event in case some other thread updates it before your “handling” has a chance to apply, i.e. you get another state update with the “one-off” event still there first, before you get your handled version.

I’m not sure if this would actually be an issue in Orbit, there’s probably an extremely slim chance of this happening due to two facts:

1. We handle state updates on a single thread
2. StateFlow conflates the updates
That said, this is theoretically possible (?). I’d have to run some tests to confirm.

To be 100% sure this doesn’t happen we’d have to allow you to reduce from the same thread that handles the one-off event i.e. the UI thread. Which kind of goes against our current design.

[Mikolaj Leszczynski]

This is all a bit funny. Antipattern #3 can also serve as an argument against one-off events in state 🙂 Because now you need to ensure immediate handling.

[Mikolaj Leszczynski]

TL;DR there is no silver bullet here. Both approaches are viable and have drawbacks.

[kioba]

The Jetpack Compose UI framework supports one off events in different forms. side- effect documentation

It is a clear indication they were evaluating this problem and decided that one off events are better described with a single shot api.

I believe based on their research it is a good practice to emulate the same behaviour within the state management solutions as well. Such as SideEffect in orbit.

PS: I'm not an expert on compose or orbit internals but both LaunchEffect and SideEffect has similar goals.

[Mikolaj Leszczynski]

Makes you wonder why they release conflicting guidance then 🙂

[kioba]

also I would add that the idea sounds novel but going from state to imperative is a difficult task and cause a lot of unintended behaviours as any update from the framework required to be reflected in the state.
I would be cautious using the same mechanism for launching bottomsheet with the current API 😅

[Guilherme Delgado]

Antipattern #2: Telling the UI to take an action

I kind of “disagree” with Manuel in this one. He suggest that “navigation” could be part of the StateUI. But I think navigation it’s more a SideEffect. Thus, in my opinion, SideEffects represent “OneOffEventState” where UiState is exclusively for UI.

Antipattern #3: Not handling the one-off event immediately

If I understood correctly the point, and so far in my experiments - while using a FSM - this didn’t became a problem, because the FSM is the one who knows/validates where you can go to, and not the “UIState” itself.
Orbit does this very well in my opinion: 2 flows for different purpose. Usually I have a LaunchedEffect listening for one-off events in my Activity/Fragment, and inside my Composables root I have another listening for UiStates:

override fun onCreate(savedInstanceState: Bundle?) {
        ...
        setContent {
            with(viewModel) {
                collectSideEffect {
                    when (it) {
                        is ShowAbout -> AlertDialog.Builder(this@LandingActivity)
                            .setTitle(R.string.dialog_about_title)
                            .setMessage(R.string.dialog_about_message)
                            .setCancelable(false)
                            .setNeutralButton(android.R.string.ok) { dialog, _ -> dialog.dismiss() }
                            .create()
                            .show()
                        is NavigateToHome -> {
                            startActivity(Intent(this@LandingActivity, MainActivity::class.java))
                            finish()
                        }
                        is NotifyWithToast -> Toast.makeText(this@LandingActivity, it.message, Toast.LENGTH_SHORT).show()
                    }
                }
                AppTheme { LandingScreen(this) }
            }
        }
    }

@Composable
fun LandingScreen(viewModel: LandingViewModel) {
    with(viewModel.collectAsState().value) {
        Landing(...)
    }
}

[appmattus]

@kioba the documentation does open with "Composables should be side-effect free" though...

[appmattus]

of course no such perfect solutions ever exist

[kioba]

yeah and they are right, same is true for any state management solution. Aim for declarative rather imperative. But in rare cases we still need to do things with a single trigger and in those rare cases it’s better to have a tool to help developers.

What I feel and others might as well that they failed to describe a declarative navigation state.
It would be still possible just not the way they suggested 😅

[appmattus]

@Mikolaj Leszczynski would Orbit not be prone to Antipattern #1? If I understand it correctly, it's not just about lifecycle but also the processing of said event - granted lifecycle will significantly reduce the risk. So even if it is received the activity/fragment could still be backgrounded and processing cancelled unless Dispatchers.Main.Immediate is used which would guarantee immediate execution - https://github.com/Kotlin/kotlinx.coroutines/issues/2886#issuecomment-901188295

[Oleksii Malovanyi]

there is no silver bullet here

can’t agree more. And while idiomatically Vivo is right in terms of classic redux-like systems, for me it seems like overkill for mobile development and this rational exists more due to implementation issues, rather than architecture benefits ¯\_(ツ)_/¯

[Oleksii Malovanyi]

but Dispatchers.Main.immediate does look like something to think about 🤔

[twisterrob]

Still a useful thread, thanks all!

@Guilherme Delgado I would argue that showing a dialog is not a (one off) side effect, because if you get it displayed like you show, and then rotate (or do anything that involves activity recreation), it'll just disappear. And it gets even more complicated when you need a confirmation dialog, not just an informational one.