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.
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