-
Notifications
You must be signed in to change notification settings - Fork 10
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[WIP] Japan example app #108
base: develop
Are you sure you want to change the base?
Conversation
exampleAppJapan/src/main/kotlin/it/trade/android/japanapp/ui/orderinput/OrderInputFragment.kt
Outdated
Show resolved
Hide resolved
btLimit.isChecked = true | ||
btMarket.isChecked = false | ||
btMarket.setOnClickListener { | ||
btLimit.isChecked = false |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any thoughts on having btLimit
and btMarket
boolean in the ViewModel vs here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
they are actually the button itself, here we're make it selected or not. however it can be made more concise or nature if keep the status in viewModel and linked them together. I'll try
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
using the model turns out a better solution. I've got a OrderType
in my OrderInfo
already.
exampleAppJapan/src/main/kotlin/it/trade/android/japanapp/ui/orderinput/OrderInputViewModel.kt
Show resolved
Hide resolved
5af6983
to
756ec02
Compare
fun increasePrice() { | ||
val value = orderForm.value | ||
orderForm.value = value?.apply { | ||
if (orderInfo.limitPrice < symbol.priceUpperLimit) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In Swift you could remove orderForm.value = value?.apply {
and replace it with orderForm?.value
, like the following:
if (orderInfo?.limitPrice < symbol.priceUpperLimit) {
orderInfo = orderInfo.copy(limitPrice = orderInfo.limitPrice + 1)
}
Can you do that in Kotlin?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes. they can be inlined. updated similar places.
} | ||
|
||
fun init(symbol: String?) { | ||
// TODO need a cleaner way to initialize the OrderForm |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
!this::orderForm.isInitialized
is needed? since this is the init of the class I assume this::orderForm.isInitialized
will return false
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
when screen rotates, Fragment will call init again. however we don't need to init the ViewModel
since we should reuse the model in this case. I'm still looking if there is a way to call this init only once and when it's needed. (need to dig into how Fragment works for this)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When configuration changes the fragment is destroyed and then recreated by the fragment manager. However the ViewModel survives configuration changes so when the activity is recreated and you call the ViewModelProviders here you actually get the same instance. https://developer.android.com/topic/libraries/architecture/viewmodel#lifecycle
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the view model factory works well. removed the init method now.
exampleAppJapan/src/main/kotlin/it/trade/android/japanapp/MainActivity.kt
Show resolved
Hide resolved
viewModel = ViewModelProviders.of(activity!!).get(OrderInputViewModel::class.java) | ||
// TODO: Use the ViewModel | ||
arguments?.getString("symbol")?.let { | ||
viewModel.init(it) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To init a ViewModel you need to create custom factory and use it with ViewModelProviders
: https://android.jlelse.eu/android-viewmodel-with-custom-arguments-d0ff0fba29e1
f56ba1e
to
f27283a
Compare
5570a92
to
45765f2
Compare
1457eee
to
ab93f6a
Compare
f4198fc
to
f96f29a
Compare
No description provided.