Is there better way than this LiveData update when the object's data has changed? ```_orderData.valu...

# codereview

[chanjungskim]

Is there better way than this LiveData update when the object's data has changed?

_orderData.value!!.checkoutResponse = newCheckoutResponse
_orderData.value = _orderData.value

[gildor]

It’s broken by definition IMO

[gildor]

checkoutResponse shouldn’t be mutable for reactive code

[gildor]

otherwise no better way and it still may cause some edge cases if consuming code doing equality check

[chanjungskim]

Well, OrderData is for keeping OrderStatus. I made like this.

@Parcelize
@Keep
data class OrderData(
    var multiCartBody: MultiCartAddRequestBody? = null,
    var checkoutBody: CheckoutRequestBody? = null,
    var checkoutResponseBody: CheckoutResponseBody? = null,
    var spreadSheetData: SpreadSheetData? = null,
    var finalPrice: Int = 0,
    var membershipDiscount: Int = 0,
    var promotionDiscount: Int = 0,
    var totalProductPrice: Int = 0,
    var membershipPrice: Int = 0,
) : Parcelable

And in an Activity, checkout API can be called multiple times. after canceling the order.
The part that I shared above is after receiving new checkout response.

[gildor]

Yep, this what I’m talking about

[gildor]

this class shoouldn’t be mutable

[gildor]

make it immutable and copy, or as alternative solution, remove data from class, make it immutable for most of fields, and for mutalbe checkoutResponse make reactive property, so client have to subscribe on it

[chanjungskim]

you mean, I need to create new class every time?
For example, _orderData.value = OrderData(newMultiCartBody, newCheckoutBody, ...) ??

[gildor]

yes, it’s also data class, you can copy it with changed fields

[chanjungskim]

_orderData.value = _orderData.value.copy( something )?

[gildor]

yes

[chanjungskim]

Okay, then, do you think the class is fine?

[gildor]

About nested reactive field I mean something like this

class OrderData(
   val orderId: String
   ...
) {
   val orderStatus: LiveData<OrderStatus>

   /*potentially can be internal or not visible for consumer
    fun updateOrderStatus(newStatus: OrderStatus) { ... }
}

[gildor]

do you think the class is fine

It’s just a data class, up to you what kind data have there, just make it immutable

[chanjungskim]

Having LiveData in a data class means, ViewModel has the OrderData. And View(Activity or XML) observes its data inside that are LiveData, right?

[gildor]

This why I suggest above to remove data from class, because it’s not really data class anymore

[gildor]

One more alternative solution is to have something like:

class Order {
   val orderData: OrderData // immutable part
} {
   val orderStatus: LiveData<OrderData>
   // any other reactive fields
}

[gildor]

And View(Activity or XML) observes its data inside that are LiveData, right?

Yes, it forces consumer View to subscribe on updates of status if they want to access it

[gildor]

But nothing wrong with having order completely immutable and copy it, different approch depending on how code is design and use case, how often it updating

[gildor]

some edge cases if consuming code doing equality check

An addition to this comment.
It will break even standard LiveData operators like distingUntilChanged, because object always equals to itself:
https://developer.android.com/reference/androidx/lifecycle/Transformations#(androidx.lifecycle.LiveData).distinctUntilChanged()

[chanjungskim]

Thank you I copy that.

[chanjungskim]

I have one more question. What does nested reactive field mean in your answer?

[gildor]

field I mean class property
nested reactive is about LiveData (which I call reactive, same as analogue of ReactiveX observable data stream)