I'm working a little bit with VIEWS and I created those following seales class.
The first one to "hold" the state for the screen, it can only be Loading, Success or Error:
sealed class ScreenUIState<out T> {
data object Loading : ScreenUIState<Nothing>()
data class Success<T>(val data: T) : ScreenUIState<T>()
data class Error(val message: Int) : ScreenUIState<Nothing>()
}
And the second one to "hold" the state of the operation:
sealed class OperationState<out T> {
object Idle : OperationState<Nothing>()
object Loading : OperationState<Nothing>()
data class Success<T>(val data: T) : OperationState<T>()
data class Error(val message: Int) : OperationState<Nothing>()
}
My question here is if this is a good practice, you know, use this like to handle every Screen or Operation:
viewModel.createInvitationOperationState.collect { state ->
when (state) {
is OperationState.Success -> {
// Handle the data or the state, like navigates to another screen
}
is OperationState.Loading -> {
// Set the loading as visible
}
is OperationState.Error -> {
// Show a snackbar message
}
else -> {
// No action //
}
}
}
I'm not sure how ScreenUIState and OperationState are related, but generic ui state holders in general usually impose an unnessecary overhead.
Not all screens will always have exactly the three states Loading, Success and Error (or a fourth, Idle, whatever that even means for the UI). If the screen has fewer states, then you burden the UI code with unused when
branches. If the screen needs more states, then you would have to nest them in one of the others. In addition, for the Success path you can only have a single object as payload. If you need more, you need to introduce another state holder that encapsulates it. And even the Error state may sometimes need more context than just an Int. (Which you probably meant to be a String. But still.)
All of this unnecessarily bloats your code and makes it harder to read and to maintain.
Instead, use custom-tailored state holders for each screen. Consider this example with a dedicated state holder:
sealed interface TopicUiState {
data object Loading : TopicUiState
data class Loaded(val topics: List<Topic>, val selected: Topic?) : TopicUiState
}
Then your composable can look something like this:
val state: TopicUiState = viewModel.uiState.collectAsStateWithLifecycle().value
when (state) {
TopicUiState.Loading -> CircularProgressIndicator()
is TopicUiState.Loaded -> TopicList(
topics = state.topics,
selected = state.selected,
selectTopic = viewModel::selectTopic,
)
}
With your generic state holder, however, it has to be more verbose:
val state: ScreenUIState<LoadedTopics> = viewModel.uiState.collectAsStateWithLifecycle().value
when (state) {
ScreenUIState.Loading -> CircularProgressIndicator()
is ScreenUIState.Success -> TopicList(
topics = state.data.topics,
selected = state.data.selected,
selectTopic = viewModel::selectTopic,
)
is ScreenUIState.Error -> Unit // not needed
}
And you need, in additon to ScreenUIState, another class to hold the actual payload:
data class LoadedTopics(val topics: List<Topic>, val selected: Topic?)
Just keep it simple, there is nothing really to gain by intrducing generics here.
Not related, but regarding the collection of the view model flow from your question (to save you some hassle later on):
Only create view model instances in the screen composables where they are needed (with viewModel()
or the respective alternative if you use a dependency injection framework) and immediately collect their flows with collectAsStateWithLifecycle()
, as shown above. Never use collect
with a lambda on the UI StateFlow.