etiennelenhart / Eiffel

Redux-inspired Android architecture library leveraging Architecture Components and Kotlin Coroutines
MIT License
211 stars 14 forks source link

Variant of updateState function using LiveData's postValue #15

Closed eugenio1590 closed 6 years ago

eugenio1590 commented 6 years ago

Hi, Excellent library to maintain the support of the UI state in the view model. I appreciate the support you have and I am currently using it in a personal project.

Even so, I am using RxJava and I see that the updateState method does not work well at all. As far as I could see, the state.value is called directly in this method. I think it would be great to have an updateState for the case of Rx using state.postValue. Everything else has worked wonders, except that method. The version of the library that I am using is 3.1.0.

Thanks.

etiennelenhart commented 6 years ago

Hey, thanks. I'm glad that you like the library. :)

I guess you're using RxJava for your business-logic "commands" and those return on a background thread? I'm not really familiar with RxJava to be honest and I personally use Eiffel together with Kotlin Coroutines. So I normally invoke updateState from a coroutine running on the UI thread inside the ViewModel.

Regardless, I already thought about adding a variant of updateState that uses postValue to make it usable from a background thread. From what I understand that would solve your issue?

Do you have any preferences concerning the API? I would go for something like below; effectively extending the current updateState function:

protected inline fun updateState(post: Boolean = false, update: (currentState: T) -> T)

That would provide you with the following call to synchronize a new state to the main thread:

updateState(post = true) { it.copy(sample = true) }

(The post = is optional, of course.)

I'm a bit hesitant to add the same to initState though, since the initial state should be set more or less immediately when the ViewModel is being initialized. Do you need any modifications there or just in updateState?

eugenio1590 commented 6 years ago

Yes, it seems perfect if you add that post parameter to indicate the status update from the bottom. And by default in false so that it is used only when you have Rx.

The initial state is not necessary. As you indicate it is done when the ViewModel is initializing and will be ready to be observed in the onCreate of the activity.

Thanks for the support.

etiennelenhart commented 6 years ago

Ok, nice. :)

I changed the issue title to better reflect your request since it is more of a general enhancement to updateState and not restricted to RxJava.

3.2.0 will solely be about updating all dependencies and packages to the new AndroidX namespaces and won't include any new features. It will be released as soon as AndroidX hits stable, so hopefully in a couple of days.

If it's not a critical feature for you, I would implement it in the next version (probably 4.0.0) which I'm planning on releasing shortly after 3.2.0.

eugenio1590 commented 6 years ago

I can wait, there's no problem. Please close the issue when the new library is available. Thanks for your help.

etiennelenhart commented 6 years ago

I created a pull request with the implementation. #17

If you want you can pull it into your project and see if it resolves your issue using JitPack's branch snapshots. You'll need the AndroidX version of Lifecycle Extensions, though and have to be on Android Studio 3.2 or higher if you're using Eiffel's Data Binding features.

implementation 'androidx.lifecycle:lifecycle-extensions:2.0.0'
implementation 'com.github.etiennelenhart:eiffel:updateState-postValue-SNAPSHOT'

I also added a note to the documentation, that states updated with post = true could be emitted after later ones updated without posting, since the main thread won't execute the update task immediately. Normally that shouldn't be much of a problem, but accessing currentState in subsequent updates may result in unexpected behavior.

I'll merge if it works for you and will release it in 4.0.0.

eugenio1590 commented 6 years ago

Hi @etiennelenhart , sorry for the delay. Everything is working well with the new library and the new AndroidX support. I suppose you will release version 3.3.x or something, right?

etiennelenhart commented 6 years ago

Hey, no problem. :) Yes, I'm currently working on the next release which will contain some breaking changes and deprecations. So it will be 4.0.x. I'm pretty confident that I'll be able to release it by the end of this week.

eugenio1590 commented 6 years ago

Excellent. I will wait for the new release. Thanks for your help.

etiennelenhart commented 6 years ago

Released in 4.0.0.