Access MutableLiveData Properties

I'm new to programming and starting to work through some tutorials in Kotlin for Android. Currently, I'm looking at data binding and view model design patterns. I know that operations on mutablelivedata should not be in the main activity, so I've moved them to the viewmodel.

I have some code that does the job, but wondering if there are better ways to do this. This is probably more of an OOP 101 question than specifically a kotlin question.

class MainViewModel: ViewModel() {
    val alarm : MutableLiveData<AlarmData> = MutableLiveData<AlarmData>(AlarmData(0, alarmType.HIGH))

    fun increaseValue(){
        val x = alarm.value
        var avalue = x!!.alarmValue
        var atype  = x!!.alarmType

        alarm.value = AlarmData(++avalue, atype)
    }

    fun decreaseValue(){
        val x = alarm.value
        var avalue = x!!.alarmValue
        var atype  = x!!.alarmType

        alarm.value = AlarmData(--avalue, atype)
    }

    fun togglePriority(){
        val x = alarm.value
        var avalue = x!!.alarmValue
        var atype  = x!!.alarmType

        when (atype){
            alarmType.HIGH -> alarm.value = AlarmData(avalue, alarmType.LOW)
            alarmType.MEDIUM -> alarm.value = AlarmData(avalue, alarmType.HIGH)
            alarmType.LOW -> alarm.value = AlarmData(avalue, alarmType.MEDIUM)
        }
    }

I tried the above code and it works, but seems very far from ideal- specifically that each function needs to use the same code to access the individual properties of the alarm object.

Solution

That is basically what you need to do, yeah - you have separate functions defining operations that are allowed, and each one needs to update the current value with a modified copy. So you will get similar code in each, handling that general read-modify-update pattern.

(The next bit is about a general approach - what I'd actually recommend in this situation is in the last part!)

One thing you can do in this general situation, is refactor common code into a shared (private) function, so you're not repeating it. For example, you could have a changeValue function that takes the current AlarmData and applies a change in value to it:

private fun changeData(alarm: AlarmData, change: Int): AlarmData {
    var avalue = alarm.alarmValue
    var atype  = alarm.alarmType

    return AlarmData(avalue + change, atype)
}

fun increaseValue() {
    alarm.value = changeData(alarm.value!!, change = 1)
}

fun decreaseValue() {
    alarm.value = changeData(alarm.value!!, change = -1)
}

So I basically moved your duplicate code into a separate function, and made the variable part a parameter on the function. And your increase/decrease functions just call that with the appropriate value. So the duplication is cut down, and it's less to maintain too!

But in this specific case, I wouldn't necessarily do that. You can cut your code down quite a bit:

fun increaseValue(){
    // define the variable as non-nullable instead, so you don't need to do it when you access it
    val current = alarm.value!!

    // and you can just access the properties directly    
    alarm.value = AlarmData(current.alarmValue + 1, current.alarmType)
}

Or you could use a scope function with a receiver to ditch the temp variable:

alarm.value = alarm.value!!.run { AlarmData(alarmValue + 1, alarmType) }

Or if you're using another Kotlin function, data classes, you can easily copy an object while making changes to it:

// define your data object as a data class, which gives you a copy() function
data class AlarmData(
    val alarmValue: Int,
    val alarmType: AlarmType
)

// now you can generate modified copies easily, specifying new values for certain properties
alarm.value = alarm.value!!.run { copy(alarmValue = alarmValue + 1) }

// or if that doesn't make sense
val current = alarm.value!!
alarm.value = current.copy(alarmValue = current.alarmValue)

So when your functions can be written in one or two lines, it might not be worth extracting a separate function - it cuts down on redundancy, but adds complexity, so it's more of a tradeoff. Your call!

Also I'll just say that working with a LiveData that has a default value (so value can never be null) is one of the few situations where I feel like it's ok to use !! in your own code, rather than null-checking it with ? and adding a scope function like run or let. So I've only used !! in these examples because I feel like it's clearer and avoids unnecessary code for the impossible null situations - but in general it's a bad habit that leads to bugs, unless you know it's necessary. Just a warning!

Oh one other thing, you usually wouldn't publicly expose the MutableLiveData like that - you expose a read-only LiveData property instead, like this:

// this mutable type stays private
private val _alarm : MutableLiveData<AlarmData> =
    MutableLiveData<AlarmData>(AlarmData(0, alarmType.HIGH))

// for the public version, you cast it to the immutable LiveData supertype
val alarm: LiveData<AlarmData> = _alarm
// or if you want to avoid creating another backing field, use a getter
val alarm: LiveData<AlarmData> get() = _alarm

This way you force consumers to use your interface (the functions you defined) to change the data, instead of being able to poke at the LiveData's value directly