https://kotlinlang.org logo
Channels
Powered by
#codereview
Title
# codereview
c

chanjungskim

01/27/2023, 2:04 AM
Is there better way than this LiveData update when the object's data has changed?
Copy code
_orderData.value!!.checkoutResponse = newCheckoutResponse
_orderData.value = _orderData.value
g

gildor

01/27/2023, 3:43 AM
It’s broken by definition IMO
checkoutResponse shouldn’t be mutable for reactive code
otherwise no better way and it still may cause some edge cases if consuming code doing equality check
c

chanjungskim

01/27/2023, 3:48 AM
Well, OrderData is for keeping OrderStatus. I made like this.
Copy code
@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.
g

gildor

01/27/2023, 3:49 AM
Yep, this what I’m talking about
this class shoouldn’t be mutable
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
c

chanjungskim

01/27/2023, 3:51 AM
you mean, I need to create new class every time? For example, 
_orderData.value = OrderData(newMultiCartBody, newCheckoutBody, ...)
??
g

gildor

01/27/2023, 3:52 AM
yes, it’s also data class, you can copy it with changed fields
c

chanjungskim

01/27/2023, 3:52 AM
_orderData.value = _orderData.value.copy( something )?
g

gildor

01/27/2023, 3:52 AM
yes
c

chanjungskim

01/27/2023, 3:52 AM
Okay, then, do you think the class is fine?
g

gildor

01/27/2023, 3:53 AM
About nested reactive field I mean something like this
Copy code
class OrderData(
   val orderId: String
   ...
) {
   val orderStatus: LiveData<OrderStatus>

   /*potentially can be internal or not visible for consumer
    fun updateOrderStatus(newStatus: OrderStatus) { ... }
}
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
c

chanjungskim

01/27/2023, 3:57 AM
Having LiveData in a data class means, ViewModel has the OrderData. And View(Activity or XML) observes its data inside that are LiveData, right?
g

gildor

01/27/2023, 3:58 AM
This why I suggest above to remove 
data
from class, because it’s not really data class anymore
One more alternative solution is to have something like:
Copy code
class Order {
   val orderData: OrderData // immutable part
} {
   val orderStatus: LiveData<OrderData>
   // any other reactive fields
}
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
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
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()
c

chanjungskim

01/27/2023, 4:40 AM
Thank you I copy that.
I have one more question. What does 
nested reactive field
mean in your answer?
g

gildor

01/27/2023, 5:57 AM
field I mean class property nested reactive is about LiveData (which I call reactive, same as analogue of ReactiveX observable data stream)
2 Views
Powered by