Skip to content
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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

DO NOT MERGE - live data & coroutines experiments #770

wants to merge 3 commits into
base: master


Copy link

yigit commented Oct 8, 2019

馃摙 Type of change

  • Bugfix
  • New feature
  • Enhancement
  • Refactoring

馃摐 Description

This PR has two parts, just sending to share some ideas.

First part is about using the liveData builder instead of launching in scopes and calling setValue on a LiveData instance.
This allows tests to just use LiveData.asFlow() instead of LiveDataTestUtils. Also much nicer organization on how the value is computed (since there is only 1 place to look)
For the test case we want 2 events, unfortunately events get conflated with LiveData.asFlow and test coroutine dispatcher executes all runnables immediately (instead of first come first serve) but adding a yield between two emits in the ViewModel solves it (not great).

The second part is replacing Event class with a regular kotlin channel.
Channels have a fan-out functionality which is explicit (unlike in SingleLiveEvent which had implicit behavior that does not match LiveData). I find it much cleaner and has room to be generalized.

I don't think it makes sense to merge this PR (hence DO NOT MERGE), but i think there is room to move more code into coroutines / flows also cleanup this PRs code.
I think even the LiveData computations can be reduced into Flows all the way until UI (viewModel's public fields that is).

馃挕 Motivation and Context

Avoiding LiveDataTestUtil.

馃挌 How did you test it?


馃摑 Checklist

  • I ran ./gradlew spotlessApply before submitting the PR
  • I reviewed submitted code
  • [] I added tests to verify changes
  • All tests passing

馃敭 Next steps

just check it

馃摳 Screenshots / GIFs

@@ -1,4 +1,4 @@

This comment has been minimized.

Copy link

nic0lette Oct 8, 2019

super nit: It looks like a space got added here accidentally. :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
None yet
3 participants
You can鈥檛 perform that action at this time.