Search code examples
androidkotlinandroid-jetpack-compose

Trigger recomposition with state objects


I'm having trouble understanding why recomposition does or does not occur. I have an object which describes a vehicle and there are two dropdowns in a dialog which allow selecting a make and a model for the vehicle. The model for the dropdowns are backed by a database query like so:

@Query("SELECT DISTINCT make FROM ${DbSchema.MakeModelSchema.TABLE_NAME}")
fun observeDistinctMakes(): Flow<List<String>>

@Query("SELECT DISTINCT model FROM ${DbSchema.MakeModelSchema.TABLE_NAME} WHERE make = :make")
fun observeModelsByMake(make: String): Flow<List<String>>

When a make is selected, I would like for the model selection to change to the first available model for that make. I've attempted this, but when changing the make within the object it does not trigger recomposition in the model Composable. Here is some minimal code for the Composables in the dialog:

@OptIn(ExperimentalMaterial3Api::class)
@Composable
fun ModelSelection(
    vinDescriptionFlow: MutableStateFlow<VinDescription>,
    previousMake: MutableState<String> //this value holds the previous make in the case of a recomposition
) {
    val vinDescriptionState = vinDescriptionFlow.collectAsState()
    val context = LocalContext.current
    val models =
        SMSRoomDatabase
            .getDatabase(context)
            .vinMakeModel
            .observeModelsByMake(vinDescriptionState.value.make)
            .collectAsState(
                initial = emptyList())
    //if the make has changed, select the first model of that make
    if(
        vinDescriptionState.value.make != previousMake.value &&
        models.value.size > 0
    ) {
        previousMake.value = vinDescriptionState.value.make
        vinDescriptionState.value.model = models.value.get(0)
    }
    val scrollState = rememberScrollState()
    Row(
        modifier = Modifier
            .fillMaxWidth()
            .height(IntrinsicSize.Max)
    ) {
        val modelLabelString = "Model"
        var expanded by remember { mutableStateOf(false) }
        ExposedDropdownMenuBox(
            modifier = Modifier
                .weight(1f),
            expanded = expanded,
            onExpandedChange = {
                expanded = !expanded
            }
        ) {
            OutlinedTextField(
                modifier = Modifier
                    .fillMaxWidth()
                    .menuAnchor(),
                readOnly = true,
                value = vinDescriptionState.value.model,
                onValueChange = {
                    vinDescriptionState.value.model = it
                },
                label = {
                    Text(modelLabelString)
                },
                placeholder = {
                    Text(modelLabelString.toUpperCase(Locale.current))
                },
                trailingIcon = {
                    ExposedDropdownMenuDefaults.TrailingIcon(expanded = expanded)
                }
            )
            ExposedDropdownMenu(
                expanded = expanded,
                scrollState = scrollState,
                onDismissRequest = {
                    expanded = false
                }
            ) {
                models.value.forEachIndexed { index, item ->
                    DropdownMenuItem(
                        text = { Text(item) },
                        onClick = {
                            vinDescriptionFlow.value.model = item
                            expanded = false
                        }
                    )
                }
            }
        }
    }
}
@OptIn(ExperimentalMaterial3Api::class)
@Composable
fun MakeSelection(
    vinDescriptionFlow: MutableStateFlow<VinDescription>,
) {
    val vinDescriptionState = vinDescriptionFlow.collectAsState()
    val context = LocalContext.current
    val makeLabelString = "Make"
    val makes =
        SMSRoomDatabase
            .getDatabase(context)
            .vinMakeModel
            .observeDistinctMakes()
            .collectAsState(
                initial = emptyList())
    var expanded by remember { mutableStateOf(false) }
    val scrollState = rememberScrollState()
    Row(
        modifier = Modifier
            .fillMaxWidth()
            .height(IntrinsicSize.Max)
    ) {
        ExposedDropdownMenuBox(
            modifier = Modifier
                .weight(1f),
            expanded = expanded,
            onExpandedChange = {
                expanded = !expanded
            }
        ) {
            OutlinedTextField(
                modifier = Modifier
                    .fillMaxWidth()
                    .menuAnchor(),
                readOnly = true,
                value = vinDescriptionState.value.make,
                onValueChange = { make ->
                    //trying to trigger recomposition by copying the state and altering flow
                    vinDescriptionFlow.value = vinDescriptionState.value.clone().also { it.make = make }
                    //vinDescriptionState.value.make = it
                },
                label = {
                    Text(makeLabelString)
                },
                placeholder = {
                    Text(makeLabelString.toUpperCase(Locale.current))
                },
                trailingIcon = {
                    ExposedDropdownMenuDefaults.TrailingIcon(expanded = expanded)
                }
            )
            ExposedDropdownMenu(
                expanded = expanded,
                scrollState = scrollState,
                onDismissRequest = {
                    expanded = false
                }
            ) {
                makes.value.forEachIndexed { index, item ->
                    DropdownMenuItem(
                        text = { Text(item) },
                        onClick = {
                            vinDescriptionFlow.value.make = item
                            expanded = false
                        }
                    )
                }
            }
        }
    }
}

And here is the dialog where they are called

@Composable
fun VinEntryDialog(
    vinDescriptionFlow: MutableStateFlow<VinDescription>,
    show: MutableState<Boolean>,
    onConfirm: (MutableStateFlow<VinDescription>) -> Unit) {
    val vinDescriptionState = vinDescriptionFlow.collectAsState()
    //clone the value and VinDescription Flow will be updated in onConfirm callback
    val localVinDescriptionFlow: MutableStateFlow<VinDescription> =
        MutableStateFlow(vinDescriptionState.value.copy())
    if(show.value)
    AlertDialog(
        onDismissRequest = {
            show.value = false
        },
        confirmButton = {
            TextButton(
                onClick = {
                    vinDescriptionFlow.value = localVinDescriptionFlow.value
                    onConfirm(localVinDescriptionFlow)
                }
            ) {
                Text("Save")
            }
        },
        dismissButton = {
            TextButton(
                onClick = {
                    show.value = false
                }
            ) {
                Text("Cancel")
            }
        },
        text = {
            val previousMake = remember { mutableStateOf(localVinDescriptionFlow.value.make) }
            Column {
                VinTextField(
                    vinDescriptionFlow = localVinDescriptionFlow)
                MakeSelection(
                    vinDescriptionFlow = localVinDescriptionFlow)
                ModelSelection(
                    vinDescriptionFlow = localVinDescriptionFlow,
                    previousMake = previousMake)
                YearTitleRow(
                    vinDescriptionFlow = localVinDescriptionFlow)
                ColorSelection(
                    vinDescriptionFlow = localVinDescriptionFlow)
            }
        })
}

What I don't understand is that the same MutableStateFlow<VinDescription> is updated in the make, and in ModelSelection the flow is collected as a state like so val vinDescriptionState = vinDescriptionFlow.collectAsState() but when that state is changed in the make vinDescriptionFlow.value = vinDescriptionState.value.clone().also { it.make = make } (also tried vinDescriptionState.value.make = it, but I think the state has to have either a different memory address or the equals function has to evaluate to false, so I cloned the object to ensure a different memory address). I also tried overriding equals, but that also seems to have no effect:

    override fun equals(compare: Any?): Boolean {
        val other = compare as VinDescription
        var equal =
            vin == other.vin &&
            make == other.make &&
            model == other.model &&
            year == other.year &&
            titleNumber == other.titleNumber &&
            hasTitle == other.hasTitle &&
            titleState == other.titleState &&
            color == other.color &&
            licenseNumber == other.licenseNumber
        if(vinImageFiles.size == other.vinImageFiles.size) {
            if(vinImageFiles.size > 0)
                vinImageFiles.forEachIndexed { index, thisImage ->
                    other.vinImageFiles.getOrNull(index)?.let { thatImage ->
                        equal = equal && thisImage.absolutePath == thatImage.absolutePath
                    }
                }
        } else {
            return false
        }
        if(remoteVinImageFiles.size == other.remoteVinImageFiles.size) {
            if(remoteVinImageFiles.size > 0)
                remoteVinImageFiles.forEachIndexed { index, thisImage ->
                    other.remoteVinImageFiles.getOrNull(index)?.let { thatImage ->
                        equal = equal && thisImage.absolutePath == thatImage.absolutePath
                    }
                }
        } else {
            return false
        }
        return equal
    }

To try to ensure that clone was operating the way I expected, I implemented my own:

    @Synchronized
    @Throws(CloneNotSupportedException::class)
    public override fun clone(): VinDescription {
        val logger = LoggerFactory.getLogger(this.javaClass)
        val vd = VinDescription()
        vinImageFiles.forEach {
            vd.vinImageFiles.add(it)
        }
        remoteVinImageFiles.forEach {
            vd.remoteVinImageFiles.add(it)
        }
        vd.vin = this.vin
        vd.make = this.make
        vd.model = this.model
        vd.year = this.year
        vd.titleNumber = this.titleNumber
        vd.hasTitle = this.hasTitle
        vd.titleState = this.titleState
        vd.color = this.color;
        vd.licenseNumber = this.licenseNumber
        logger.info("cloning complete!")
        return vd
    }

And the VinDescription constructor looks like this:

data class VinDescription(
    var vin: String = "",
    var make: String = "",
    var model: String = "",
    var year: String = "",
    var titleNumber: String = "",
    var hasTitle: String = "",
    var titleState: String = "",
    var color: String = "",
    var licenseNumber: String = "",
    var vinImageFiles: MutableList<File> = mutableListOf(),
    var remoteVinImageFiles: MutableList<File> = mutableListOf()
)

So my understanding is that altering the object in a MutableStateFlow should trigger recomposition because the MutableStateFlow<VinDescription> is being collected using collectAsState(), so why is that collect not triggering recomposition when the value of the flow is being changed? A call to emit() should occur and the new state should be collected in ModelSelection, right? What am I missing?


Solution

  • There are several issues with your code, mostly regarding the immutability of your state objects.

    You should only pass simple, immutable objects to your composables. Only pass the value of State and Flow objects, not the objects themselves.

    Also, you don't need a custom clone or equals method. Kotlin automatically creates appropriate methods if you declare your class as a data class. In addition, VinDescription should be an immutable data class, all properties should be declared as val not as var. If you need to change a property, just create a copy of the objects with that property changed. All data classes have a copy method (the replacement for the Java's clone method) that is meant to be used for that. Simply replace

    vinDescriptionState.value.clone().also { it.make = make }
    

    with

    vinDescriptionState.value.copy(make = make)
    

    Also make sure The lists in VinDescription are immutable:

    val vinImageFiles: List<File> = emptyList(),
    val remoteVinImageFiles: List<File> = emptyList(),
    

    If you want to add a new element to the list, simply use the + operator:

    val newFile: File
    val list = vinDescriptionState.value.vinImageFiles
    val newList = list + newFile
    

    That creates a new list containing all previous elements plus newFile. You can also use - and several other operators.

    All of this may seem cumbersome or inefficient at first glance but it is quite efficient and won't measureably impact performance. But it will satisfy Compose's requirement for immutable states that is necessary to properly detect state changes.

    You may also want to consider using view models instead of directly accessing the database, that simplifies matters a lot.


    Addressing the comments

    Compose is state-driven. To properly handle state changes Compose needs all mutable state to be an androidx.compose.runtime.State. Mutability is achieved by updating this State object which will be registered by the Compose runtime and triggers a recomposition. Directly mutating only the content of the State object will bypass that and the Compose runtime cannot detect that change and cannot properly update the UI.

    From that follows that the object you store in such a State must be immutable. That's one of the basic principles of Compose.

    Moreover, shared mutable state is generally frowned upon (not just in Compose, that's a major issue in many programming languages; also see Mutable vs immutable objects). You should never have multiple parts of your app access mutable data because it is so difficult to correctly propagate notifications about changes to the data. This will also never work properly when multiple threads are involved.

    Instead you should change your overall architecture to have a clearly defined Single source of truth. That source needs to publish its data as immutable objects (data classes with val). If the data should be able to change over time simply wrap the data in a Flow. Each time a new value is available at the souce it just emits a new (also immutable) object into the fow. This also helps enforcing the Separation of concerns.

    All of that is independent of Compose and should be done one way or another, but if you have structured your data sources and business logic to follow these principles of clean architecture then it also becomes real easy to put a Compose UI on top. That's not so much an issue of Compose as it is an issue with your architecture.

    For data that may change over time only pass Flows through the layers of your app (by adhering to Unidirectional data flow, i.e. bottom-up). Your business logic only transforms the flows and passes them on until they reach your view model where they are converted to StateFlows (with stateIn). As soon as a composable retrieves its view model it collects the flow (using collectAsStateWithLifecycle, use by delegation to also unwrap the State) so you will never have the flow lying around. You will then only pass the collected value (which should be an immutable instance of a data class) to other composables.

    A last word on data classes:
    All properties defined in the primary constructor of a data class must be immutable because these properties are used in the auto-generated equals, hashCode and copy functions (for more see https://kotlinlang.org/docs/data-classes.html). You can have additional mutable properties as you like as long as they are only used internally (i.e. they are private). That won't help much, though, because whatever functions you have defined that operate with these internal properties, they won't have any effect on the publicly available val properties. Which was the goal from the start, otherwise the class wouldn't be immutable.