androidkotlinmvvmandroid-jetpack-composedagger-hilt

How to follow MVVM patterns for storing a TextField's value when using a sealed class UiState in the view model?


I've made an Android app with Kotlin + Room + Hilt + MVVM, etc. I've made a screen where I can create or edit players, for that I've made the composables, a view model, and a sealed class representing the UiState.

It works OK, but I have two variables (name and alias) with rememberSaveable in the composable which I guess they should be in the view model or integrated in the UiState. Can you help me with that? Is my code OK or should I improve it following MVVM moving those variables? Which is the best practice?

This is the code:

UiState

sealed class PlayerUiState {
    data object Loading : PlayerUiState()
    data object Creation : PlayerUiState()
    data class Edition(val player: Player) : PlayerUiState()
    data class Error(val message: String) : PlayerUiState()
}

View Model

@HiltViewModel
class PlayerViewModel @Inject constructor(
    private val playerRepository: PlayerRepository,
    @ApplicationContext private val context: Context,
) : ViewModel() {
    private val _uiState = MutableStateFlow<PlayerUiState>(PlayerUiState.Loading)
    val uiState: StateFlow<PlayerUiState> = _uiState.asStateFlow()

    fun setState(uiState: PlayerUiState) {
        _uiState.value = uiState
    }

    fun getPlayer(playerId: Int) {
        viewModelScope.launch {
            playerRepository.getPlayer(playerId).onStart {
                setState(PlayerUiState.Loading)
            }.catch { e ->
                setState(
                    PlayerUiState.Error(
                        e.message ?: context.getString(R.string.msg_unknown_error)
                    )
                )
            }.collect { player ->
                setState(PlayerUiState.Edition(player))
            }
        }
    }

    fun insertPlayer(name: String, alias: String) {
        viewModelScope.launch {
            playerRepository.insertPlayer(
                Player(
                    name = name,
                    alias = alias,
                )
            )
        }
    }

    fun updatePlayer(player: Player) {
        viewModelScope.launch {
            playerRepository.updatePlayer(player)
        }
    }
}

Composables

@Composable
fun PlayerScreen(
    modifier: Modifier = Modifier,
    titleRes: Int,
    viewModel: PlayerViewModel = hiltViewModel(),
    playerId: Int? = null,
) {
    val uiState by viewModel.uiState.collectAsState()
    LaunchedEffect(playerId) {
        if (playerId != null) {
            viewModel.getPlayer(playerId)
        } else {
            viewModel.setState(PlayerUiState.Creation)
        }
    }
    PlayerScreenContent(
        modifier = modifier,
        titleRes = titleRes,
        uiState = uiState,
        onInsert = { name, alias -> viewModel.insertPlayer(name, alias) },
        onUpdate = { name, alias ->
            val player = (uiState as PlayerUiState.Edition).player
            viewModel.updatePlayer(player.copy(name = name, alias = alias))
        },
    )
}

@Composable
fun PlayerScreenContent(
    modifier: Modifier = Modifier,
    titleRes: Int,
    uiState: PlayerUiState = PlayerUiState.Loading,
    onInsert: (String, String) -> Unit,
    onUpdate: (String, String) -> Unit,
) {
    Scaffold() { innerPadding ->
        Box(
            contentAlignment = Alignment.Center,
            modifier = Modifier
                .fillMaxSize()
                .padding(innerPadding),
        ) {
            when (uiState) {
                is PlayerUiState.Loading -> CircularProgressIndicator()
                is PlayerUiState.Error -> ErrorDialog(
                    text = uiState.message,
                )

                is PlayerUiState.Creation -> PlayerForm(
                    onSave = onInsert,
                )

                is PlayerUiState.Edition -> PlayerForm(
                    player = uiState.player,
                    onSave = onUpdate,
                )
            }
        }
    }
}

@Composable
fun PlayerForm(
    player: Player? = null,
    onSave: (String, String) -> Unit,
) {
    var name by rememberSaveable { mutableStateOf(player?.name ?: "") }
    var alias by rememberSaveable { mutableStateOf(player?.alias ?: "") }

    Box(
        modifier = Modifier
            .fillMaxSize()
            .padding(16.dp),
        contentAlignment = Alignment.BottomCenter,
    ) {
        Column(
            modifier = Modifier.align(Alignment.Center),
            horizontalAlignment = Alignment.CenterHorizontally,
            verticalArrangement = Arrangement.spacedBy(8.dp),
        ) {
            OutlinedTextField(
                value = name,
                onValueChange = { name = it },
                label = { Text(stringResource(id = R.string.text_field_name)) },
            )
            OutlinedTextField(
                value = alias,
                onValueChange = { alias = it },
                label = { Text(stringResource(id = R.string.text_field_alias)) },
            )
        }
        Row(
            modifier = Modifier
                .fillMaxWidth()
                .padding(16.dp)
                .align(Alignment.BottomCenter),
            horizontalArrangement = Arrangement.SpaceEvenly,
        ) {
            Button(
                onClick = { onSave(name, alias) },
                enabled = name.isNotEmpty(),
            ) {
                Text(stringResource(id = R.string.button_save))
            }
        }
    }
}

Solution

  • It's ok as it is.

    There are two variants of TextFields, one uses a String for its value, the other uses a TextFieldValue. You use the variant that uses a String, and that String needs to be wrapped by a State, otherwise the TextField wouldn't recompose when it is changed.

    You already have that:

    var name by rememberSaveable { mutableStateOf(player?.name ?: "") }
    var alias by rememberSaveable { mutableStateOf(player?.alias ?: "") }
    

    Now, should this be moved to the view model? Since view models shouldn't contain any MutableState, the only alternative is to change this to MutableStateFlows so that they can be converted to States later on by using collectAsState() in your composables.

    But this is something you also should not do: Don't use asynchronous data structures (which MutableStateFlows are) to store TextField values. You can read more on this topic in Effective state management for TextField in Compose.

    That leaves you with no options to hold the single source of truth of a TextField's value in a view model. Therefore, it must stay in the composable, as you already have it.


    The future looks a little bit different: From Compose version 1.7.0 onwards there is a new BasicTextField variant (TextField and OutlinedTextField are the Material 3 flavors that are built on top of BasicTextField). This new variant takes a TextFieldState that remedies several limitations of the other variants. Developers are encouraged to switch to this variant from the variants using String and TextFieldValue. But this is not yet done for TextField and OutlinedTextField, so you have to wait a little longer before you can replace your rememberSaveable { mutableStateOf(...) } with rememberTextFieldState().