Sealed Classes for UI State are an ANTI-PATTERN - Here's why!
Vložit
- čas přidán 6. 04. 2024
- In this video you'll find out why Sealed Classes for UI State are an ANTI-PATTERN!
💻 Let me be your mentor and become an industry-ready Android developer in 10 weeks:
pl-coding.com/drop-table-ment...
⭐ Courses with real-life practices
⭐ Save countless hours of time
⭐ 100% money back guarantee for 30 days
⭐ Become a professional Android developer now:
pl-coding.com/premium-courses/
Get my FREE PDF about 20 things you should never do in Jetpack Compose:
pl-coding.com/jetpack-compose...
Regular programming advice on my Instagram page: / _philipplackner_
Join my Discord server:
/ discord - Věda a technologie
As a general rule, avoiding inheritence leads to better (simpler) abstractions. (sealed) Interfaces, data classes, objects and functions are enough to build anything you want.
Agree, when needs are specific one or max two levels of abstractions are enough. no need to create complex code to show for good we are. Wisdom is to describe complex things in simple terms.
Your title is a bit click-baity, even though you have a valid point. Usually, I'd either use a data class as ui state with sealed class properties or - which is even better - combine flows and build the ui state from them.
Sealed classes for UI state are not an anti-pattern, incorrect modelling of the state is :D
Great example of a situation where composition is much better than inheritance. Progressive and useful, thank you!
5:20 I don't recommend using the type casting at all.
A better way is to use a local variable to capture the value of state
For example:
val currentState = state
state = when (currentState) {
is ProfileState.LocalUser -> state.copy(isLoading = !currentState.isLoading)
// same for RemoteUser
}
But your suggestion of using interface is good.
yep, you could even move the 'currentState' variable inside the when block like 'state = when (val currentState = state) {...}'
Recently had a case similar to that. The state was stored as Sealed class only, so when a common state was needed to be introduced, you had to write that thing to everywhere. Changed to your demonstrated version. It is clearly much better
Awesome tip. Great eye opener
I don't think it's that much better using sealed interface. I feel like sealed class (or interface) isn't as good for UI state because it makes the dev approach the UI with the mindset that every state is its own UI, that they are somehow distinct while mostly you just want to make some fields different, show different configuration of the same thing. I feel like making a State data class inside a ViewModel with its own fields is much more flexible approach for general UI implementation. Sealed class imo makes more sense for more specific things.
you can use sealed class/interface and specifically properties that will exist for all states and selaled is better to represent loading (missing) data than making fields nullable or empty
I have not come across that problem using sealed classes yet and i dont have a paramter for my sealed classes, they are parameter-less and inside i have data objects and data classes that represents isLoading, dataFetched, error etc.
Arrow Optics would also solve the issue, but fewer engineers are familiar with it, the syntax looks strange, and you'd need to add annotations to the properties.
One more option I prefer is to use Sealed Class to only represent the state, And have a separate data class to store any data
This sounds interesting. Can you share an example?
@@sabakathawala3038
Have a sealed class to represent the UI state independent of data
sealed class MainState {
object Loading : MainState()
object Success : MainState()
object Error : MainState()
}
and have data class to store the actual data
data class State (
val successData: Data,
val errorData: Error
)
now you don't have to do type gymnastics to get the data from a particular state, you can access successData even in errorState
Hey Philipp, can you please make one detailed video on compose multi-platform, using the latest techniques, including all the new Android ViewModel best practices, starting from scratch?
would be really great if you could do this.
Thanks in advance!
Isn't that a composition over inheritance kinda type of change?
The better approach is to create a viewModel that hosts these shared properties, and then create delegate classes for local and remote users, I think that's a better approach in general because of readability issues (delegates better than inheritance in this case)
Okay, what about stability now? Our `data class` became Unstable after adding an interface to it. Do you recommend enforcing that interface and class to be stable with `@Stable` annotation
Even a simple List will make your state class unstable which most UI states have and don't care about 😅
You can enforce it of course if you're sure that it's stable, but in general I'd only work on such details if they really end up being a (noticeable) problem. Otherwise pre-mature optimization
@@PhilippLackner I disagree. State class is the most heavy class in a given screen with huge comparison operators. Making it unstable would prevent recompositions at the right time. The correct way is to annotate the state as Immutable or use a parent marker interface, which is then added to compose stability configuration file.
hello
this is something related a your video about Viewmodels with view 2 ways to states data classes vs sealed class ???
i cant' remmember name of video....
Using Sealed Interface is good, i think when we doesn't need a parameter for model our State.
great video, however still i believe sealed classes are preferred when you have simple distinct variations of your UI state, also may I know why you always use compose stateOf for state handling, isn't stateFlow generally preferred for real world MVI applications?
If I need reactivity, I use extra state flows, but other than that there's no gain in it which is why the exposed state I always use Compose state for
@@PhilippLackner based, and there's no need to save each property to a separate variable
I have a qualm with your criticism of the entire management of the state by the viewmodel. The solution is something much more scalable: instead of modifying the UI state - which is resolved and detailed, you should modify individual pieces. I.e. a mutable state flow which tracks individual booleans and individual pieces of non-UI state. Then the main state is simply a resolver (combine operation) over all those flows. The resolver takes the current state of all the pieces, and constructs the full detailed UI state upon every change. This is much more scalable especially for more complex UI, and allows you to pull more logic into the VM from the view. It also makes sure you are less likely to produce an invalid UI state - i.e. if you modify the isLoading flag, is there some other property in the state which you forgot to update? If you have a certral place where you construct the entire UI state, then you are very likely to always produce the right UI state.
Thanks for the feedback and sharing that :) it's also a viable solution, so thanks for sharing
what in case you have lets say screen ui state with just 3 states such as Loading, Error and Loaded. they do not share any data/variables, isn't sealed class good for such case?
yeah i already experience, now im using sealed interface as feature flag
its feel good if u have multiple feature in one screen
I generally use sealed classes for navigation for containing the paths
I think should combine all properties of these UI state class to one single class, then observe every single property one by one on presentation(compose/androidView) or better than that, split state, done use sealed class/interface like your case
That's what I suggest, putting states in a data class is fine, we don't need to over complicate, for our simple todo list apps😂😂😂
I feel like you narrowed down the scope of the issue way too much. I don't think this is a "UI" problem at all, it's just the generic case for sealed classes in any context. Nevertheless it was a good explanation!
I really like the overall message and solution, but I think using mutable state over mutable state flow is also an anti-pattern here. With `MutableStateFlow` you get an `update` method which gives you that atomic mutation operation you're missing with `MutableState`, and that was causing much of the pain. Also using `by mutableState` seems to have no benefit over a plain property. Android docs also push for StateFlow usage.
class ProfileViewModel : ViewModel() {
private val _state = MutableStateFlow(ProfileState.Local)
val state = _state.asStateFlow()
fun toggleFollow() {
_state.update { lastState ->
when (lastState) {
ProfileState.Local -> lastState.copy(...
ProfileState.Remote -> lastState.copy(...
}
}
}
}
We can use this code instead of "as":
state = when (val currentState = state) {
i prefer combine with different flows and then build a UI state
Will it be serializable?
Just by reading the title of your video gives a different idea of what you actually explain. I agree with the content, but the title is misleading. People just reading the title will understand your point differently and you'd be spreading misinformation. I'd suggest using less click-bait video titles and focusing more on improving the Android community, not on only getting more views.
Your example of not being able to use .copy on the sealed class can be handled by providing a method in the base class to expose functionality .copy provides on the implementers.
I also think you could have simplified the view-model using `private val MutableStateFlow` and exposing the .asStateFlow to the view which would remove the issues you demoed related to type-casting and var.
I wouldn't typically use a sealed class like that but I think your "bad example" was a little unfair and the drawbacks you mentioned could have been handled differently.
Imagine having to extend the copy function everytime ANY property is changed/added/removed in any of the subclasses. Sorry, but that's not a good solution 😅
And what you use as a state holder doesn't have anything to do with the type casting, that will still be necessary with StateFlow. You'll still need to do
_stateFlow.update {
when(it) {
ProfileState.Local -> ...
ProfileState.Remote -> ...
}
}
does that apply to sealed interfaces?
It seems in this scenario that 'posts' are the same for 'Local' and 'Remote' user. I do not see how that can be. If it is the case that you have to fetch 'posts' depending on the user-role, why bother to use the same ProfileState?
Well, let's say that 'posts' are the same for Local and Remote. Then it makes sense to have a common 'isLoading' state.
But still that would be the only thing in common? Is it worth it, this try to make generic? (I know I love the principle, but this just makes me a bit doubtful)
Anyway, it is totally convincing that using a sealed class in the UI like in this example is a mistake.
I think your misunderstanding is to think that the posts for remote and local user have to be the same ones, which they don't have to. What this shared posts property means is that no matter if we're inspecting a local or remote user profile, there is some sort of posts list since every user has one. But depending on the user they differ of coursw
This excellent man made me realize the flaws in my coding, please keep moving forward
1. Your View should have an UIModel class with some primitives. For Example if your View has title and subTitle, UiModel should be class UiModel(val title:String, val subTitle:String). Then you may map any object to the UI model.
2. Do not use directly fun copy. You will not able to find all usages of copy in your code
Create abstract fun changeLoadingTo(isLoading:Boolean): ProfileState{ return copy(isLoading = isLoading)}
Sealed classes don't have a copy function, only data classes do
@@PhilippLackner Yes... and do not use copy directly. Create a fun into your data class for change a state to new one. That will allow you to add a checks in the fun.
There's nothing wrong with using copy directly, the changeLoadingTo function you wrote doesn't do anything
@@PhilippLackner What method/approach will you use to find all usages of copy fun in your code? It will be very difficult. But I will to it fast , by finding all usages of my changeLoadingTo function
And the function copy in ANTI-pattern by DDD and OOP. The function `copy` has neither physical meaning nor business meaning
Ty
The example is so wrong in so many places it completely misses to illustrate the point. If you have a loading - it's a state, use at as one: data object Loading : ProfileState().
If you have shared properties for _all_ of your states - use abstract value in your parent sealed class and override it in your subclasses: data class LocalUser(override val posts: List).
3rd problem with smartcasting is an easy fix, just use when (val s = state) {... and use s.copy() or whatever you need, it would be smartcasted.
Also, you don't need to cast in case of toggle if you pass that model to your VM, since it's required to operate with this method.
I like your vids, but here I disagree completely.
Thank you for adding the part about when(val s = state), that's correct which I didn't think of here. It gets rid of the inner type-casts but not the outer ones, but the issue remains that you can't update the shared properties without a type-check. That's also the case if you use abstract values, since copy() only works for data classes which the outer sealed class is not
@@PhilippLackner you are right about the copy. I didn't have a case where I have to create a copy of the state itself without creating a substate directly (where I can use a copy).
In that case you have to add a copy method to your base sealed class by yourself. A bit dirty, but at least the casting would be in one place in the code.
Why do you have business logic in the UI state - that is the question. Edit: Ok, so in the end you made it flat, looks much better. Usually when we have a similar problem to solve - we create state mutators to keep ViewModel cleaner.
What exactly is business logic here?
Dude what are you taking about ?
I would keep posts and isLoading as seperate mutableStateOf values in the viewmodel, then you could change those as needed, and get rid of .copy altogether
You still used sealed interfaces, you just used a better abstraction for the problem than the first time
Yup my point was just to not use them for the actual ui state class
The plan I had in my mind was to have a variable in ProfileState as userType or get the userType from navArgs or preferences.
Then based on the userType consider either isFollowing or isUpdatingProfilePicture.
That's an option as well, however it doesn't communicate by design which properties are specific to which state
@@PhilippLackner ohh. That's true. I didn't think that way. If I write such code, only i would know the use of it but not the other devs.
This was a clickbait title for a video about bad UI state modeling, using inheritance where composition should have been used.
Sealed classes on their own are far from anti-pattern when modeling UI state. For example, if you have states like Loading, Result and NoData, where you have three different states that have almost nothing in common. For example, showing a full screen loader when fetching the data, then showing the data, or showing a message that there is nothing yet.
What was shown here indeed was an anti-pattern, but the anti-pattern was using inheritance over composition, and it is much wider topic than modeling UI states in mobile apps.
The better way to store the state inside private variables and call some function to build a UI State with Sealed Classes.
Hello, can you share example?
Good contents. Bad title.(not related to sealed at all)
Cool