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

View actions (snackbar, activity navigation, ...) in ViewModel #63

Open
fabioCollini opened this Issue Jun 20, 2017 · 66 comments

Comments

Projects
None yet
@fabioCollini

Sometimes the ViewModel needs to invoke an action using an Activity (for example to show a snack bar or to navigate to a new activity). Using MVP it's easy to implement it because the presenter has a reference to the Activity. The ViewModel doesn't contain a reference to the Activity, what's the best way to implement this feature?
In the GithubBrowserSample you created a method getErrorMessageIfNotHandled to solve this problem, are there other solutions?
In a demo project I am working on I have created a UiActionsLiveData class that allows the ViewModel to invoke an action on the view. The connection between the ViewModel and the view is managed using a LiveData so the ViewModel doesn't contain a reference to the View even if it can invoke actions on it. The usage is simple, the ViewModel can add an action to the UiActionsLiveData:

uiActions.execute { navigationController.showError(it, t.message) }

Thanks to the LiveData implementation the action will be executed on the Activity when it's resumed.
I like this solution but it's a bit complicated, an official solution would be great.

@JoseAlcerreca

This comment has been minimized.

Show comment
Hide comment
Collaborator

JoseAlcerreca commented Jun 21, 2017

@burntcookie90

This comment has been minimized.

Show comment
Hide comment
@burntcookie90

burntcookie90 Jun 21, 2017

I think using SingleLiveEvent as a basis for other one off (notification style) events works great. I've used this pattern in a similar (internal) framework with an older project. Deploying it as part of the library makes sense, allows us all to not have to reinvent the wheel 😄 .

I think using SingleLiveEvent as a basis for other one off (notification style) events works great. I've used this pattern in a similar (internal) framework with an older project. Deploying it as part of the library makes sense, allows us all to not have to reinvent the wheel 😄 .

@fabioCollini

This comment has been minimized.

Show comment
Hide comment
@fabioCollini

fabioCollini Jun 21, 2017

In my implementation I am saving a list of events but in the SingleLiveEvent class you are managing just a single value. For example if the ViewModel produces more than a value when the Activity is paused the observer will get only the last value produced. Is this an expected behaviour? I would expect to see all the value on the observer (then the View can decide to ignore some values).

In my implementation I am saving a list of events but in the SingleLiveEvent class you are managing just a single value. For example if the ViewModel produces more than a value when the Activity is paused the observer will get only the last value produced. Is this an expected behaviour? I would expect to see all the value on the observer (then the View can decide to ignore some values).

@JoseAlcerreca

This comment has been minimized.

Show comment
Hide comment
@JoseAlcerreca

JoseAlcerreca Jun 22, 2017

Collaborator

Interesting, for navigation it makes no sense to hold a list and it's officially not recommended for Snackbar but you can join multiple messages in one. We'll take it into account thanks!

Collaborator

JoseAlcerreca commented Jun 22, 2017

Interesting, for navigation it makes no sense to hold a list and it's officially not recommended for Snackbar but you can join multiple messages in one. We'll take it into account thanks!

@sockeqwe

This comment has been minimized.

Show comment
Hide comment
@sockeqwe

sockeqwe Jun 22, 2017

A big NO from my side for SingleLiveEvent!
Unidirectional Dataflow and immutability ftw! (also a little state reducer over here and there doesn't hurt)

SearchViewModel which offers a LiveData<LoadMoreState> getLoadMoreStatus() method for the view to subscribe / listen to.
So once the view has displayed the error message (i.e. with a snackbar) the View calls:

viewModel.setLoadMoreErrorShown();

This will then force the viewModel to emit a new (immutable) LoadMoreState.

class SearchViewModel extends ViewModel {
   private MutableLiveDate<LoadMoreState> loadMoreState;
   ...
   
   // Subscribed by the View
   public LiveData<LoadModeState> getLoadMoreStatus() {
       return loadMoreState;
    }

   public void setLoadMoreErrorShow(){
         loadMoreState.setValue(new LoadMoreState(false, null)); // null means no error message to display , false means don't display loading more progress bar
    }
}

behaves like SingleLiveEvent (i.e. after screen orientation change, snackbar wont show again, because LoadMoreState.errorMessage == null) but is immutable!

sockeqwe commented Jun 22, 2017

A big NO from my side for SingleLiveEvent!
Unidirectional Dataflow and immutability ftw! (also a little state reducer over here and there doesn't hurt)

SearchViewModel which offers a LiveData<LoadMoreState> getLoadMoreStatus() method for the view to subscribe / listen to.
So once the view has displayed the error message (i.e. with a snackbar) the View calls:

viewModel.setLoadMoreErrorShown();

This will then force the viewModel to emit a new (immutable) LoadMoreState.

class SearchViewModel extends ViewModel {
   private MutableLiveDate<LoadMoreState> loadMoreState;
   ...
   
   // Subscribed by the View
   public LiveData<LoadModeState> getLoadMoreStatus() {
       return loadMoreState;
    }

   public void setLoadMoreErrorShow(){
         loadMoreState.setValue(new LoadMoreState(false, null)); // null means no error message to display , false means don't display loading more progress bar
    }
}

behaves like SingleLiveEvent (i.e. after screen orientation change, snackbar wont show again, because LoadMoreState.errorMessage == null) but is immutable!

@fabioCollini

This comment has been minimized.

Show comment
Hide comment
@fabioCollini

fabioCollini Jun 23, 2017

I agree that unidirectional dataflow and immutability are great but in this example I have a doubt about your solution: you update a LiveData while you are updating the view because of a modification of the same LiveData. At the beginning of dispatchingValue method there is a check to avoid observer nested invocations, will it work correctly? Are there other cases when this double update of the view can be a problem? For example can you manage more than one field in this way? Or what about multiple updates of the same field when the activity is paused, will the state contain a list instead of a single value?
However the two solutions are quite similar, in my opinion a SingleLiveEvent is not a violation of immutability and unidirectional dataflow. In your example the view invokes a method on the ViewModel to update the LiveData, using a SingleLiveEvent the LiveData is updated in a similar way automatically.

I agree that unidirectional dataflow and immutability are great but in this example I have a doubt about your solution: you update a LiveData while you are updating the view because of a modification of the same LiveData. At the beginning of dispatchingValue method there is a check to avoid observer nested invocations, will it work correctly? Are there other cases when this double update of the view can be a problem? For example can you manage more than one field in this way? Or what about multiple updates of the same field when the activity is paused, will the state contain a list instead of a single value?
However the two solutions are quite similar, in my opinion a SingleLiveEvent is not a violation of immutability and unidirectional dataflow. In your example the view invokes a method on the ViewModel to update the LiveData, using a SingleLiveEvent the LiveData is updated in a similar way automatically.

@sockeqwe

This comment has been minimized.

Show comment
Hide comment
@sockeqwe

sockeqwe Jun 23, 2017

At the beginning of dispatchingValue method there is a check to avoid observer nested invocations

Well, I wasn't aware of this implementation detail. I was just saying that to me invoking something like setLoadMoreErrorShown() seems more correct, transparent and easier to test compared to SingleLiveData. I'm wondering if one could implement it's own LiveData class without this check (or at least allow it if the value to dispatch has changed)

For example can you manage more than one field in this way?

I cant see why not.

Or what about multiple updates of the same field when the activity is paused, will the state contain a list instead of a single value?

No, just the latest value, not a list. In contrast to MVP, in MVVM ViewModels typically have this fields to observe a certain value. I cant find a use case where I need to.keep track of previous undispatched values. Do you?

in my opinion a SingleLiveEvent is not a violation of immutability and unidirectional dataflow.

I disagree, it clearly is a side effect (not a pure function) and obviously not immutable and not a unidirectional dataflow (state is not changed and maintained from one component).
However, if SingleLiveData works for you and then sure go ahead an use it. At the end of the day no user of your app cares about immutability and unidirectional data flow. I was just saying that from my point of View SingleLiveData violates some core principles (like immutability) and that this problem of a snackbar could be solved more idiomatically i.e. viewModel.setLoadMoreErrorShown, but maybe SingleLiveData is a pragmatic solution... practical vs. idealistic ...

At the beginning of dispatchingValue method there is a check to avoid observer nested invocations

Well, I wasn't aware of this implementation detail. I was just saying that to me invoking something like setLoadMoreErrorShown() seems more correct, transparent and easier to test compared to SingleLiveData. I'm wondering if one could implement it's own LiveData class without this check (or at least allow it if the value to dispatch has changed)

For example can you manage more than one field in this way?

I cant see why not.

Or what about multiple updates of the same field when the activity is paused, will the state contain a list instead of a single value?

No, just the latest value, not a list. In contrast to MVP, in MVVM ViewModels typically have this fields to observe a certain value. I cant find a use case where I need to.keep track of previous undispatched values. Do you?

in my opinion a SingleLiveEvent is not a violation of immutability and unidirectional dataflow.

I disagree, it clearly is a side effect (not a pure function) and obviously not immutable and not a unidirectional dataflow (state is not changed and maintained from one component).
However, if SingleLiveData works for you and then sure go ahead an use it. At the end of the day no user of your app cares about immutability and unidirectional data flow. I was just saying that from my point of View SingleLiveData violates some core principles (like immutability) and that this problem of a snackbar could be solved more idiomatically i.e. viewModel.setLoadMoreErrorShown, but maybe SingleLiveData is a pragmatic solution... practical vs. idealistic ...

@ZakTaccardi

This comment has been minimized.

Show comment
Hide comment
@ZakTaccardi

ZakTaccardi Jun 25, 2017

I didn't read this in too much detail, so sorry if I'm off base.

But the snack bar is a pretty simple solution. Its timed dismissal is just another form of input from your UI

I didn't read this in too much detail, so sorry if I'm off base.

But the snack bar is a pretty simple solution. Its timed dismissal is just another form of input from your UI

@JoseAlcerreca

This comment has been minimized.

Show comment
Hide comment
@JoseAlcerreca

JoseAlcerreca Jun 26, 2017

Collaborator

@sockeqwe since your blog post has attracted so much attention, can you confirm the following? It seems that your solution:

  1. requires more logic in the activity (onDismissed listener and a null check)
  2. setLoadMoreErrorShow should be called resetLoadMoreState, or to use the original,setLoadMoreShown, not sure why you added "Error" there.
  3. it behaves differently than SingleLiveData, since you're resetting after dismissal while we have a "fire and forget" approach. I understand you want to store state but there could be a case where the app is opened days after the last use and the user sees a misleading error message from 2 days ago. It might not be a good fit for a Snackbar. It is a better fit for a navigation event, for example.

As already rectified on your blog post, SingleLiveEvent doesn't use null internally. So statements like

state is not changed and maintained from one component

are not true in this case (if I understand you correctly). You thought the activity was setting the event's data to null. Only the ViewModel, IIRC, sets the value. We considered using composition instead of inheritance to protect who sets the value but we would have to proxy things like observeForever().

Collaborator

JoseAlcerreca commented Jun 26, 2017

@sockeqwe since your blog post has attracted so much attention, can you confirm the following? It seems that your solution:

  1. requires more logic in the activity (onDismissed listener and a null check)
  2. setLoadMoreErrorShow should be called resetLoadMoreState, or to use the original,setLoadMoreShown, not sure why you added "Error" there.
  3. it behaves differently than SingleLiveData, since you're resetting after dismissal while we have a "fire and forget" approach. I understand you want to store state but there could be a case where the app is opened days after the last use and the user sees a misleading error message from 2 days ago. It might not be a good fit for a Snackbar. It is a better fit for a navigation event, for example.

As already rectified on your blog post, SingleLiveEvent doesn't use null internally. So statements like

state is not changed and maintained from one component

are not true in this case (if I understand you correctly). You thought the activity was setting the event's data to null. Only the ViewModel, IIRC, sets the value. We considered using composition instead of inheritance to protect who sets the value but we would have to proxy things like observeForever().

@sockeqwe

This comment has been minimized.

Show comment
Hide comment
@sockeqwe

sockeqwe Jun 27, 2017

@JoseAlcerreca thanks for your feedback. It's hard to describe this in detail with text only. I have forked the Github client sample and will implement the repo search with what I have in mind. I hope that a concrete code example makes my point more understandable and a better basis for the future discussion, don't you think so? Will work on it on Thursday...

sockeqwe commented Jun 27, 2017

@JoseAlcerreca thanks for your feedback. It's hard to describe this in detail with text only. I have forked the Github client sample and will implement the repo search with what I have in mind. I hope that a concrete code example makes my point more understandable and a better basis for the future discussion, don't you think so? Will work on it on Thursday...

@svasilinets

This comment has been minimized.

Show comment
Hide comment
@svasilinets

svasilinets Jun 27, 2017

Collaborator

At the beginning of dispatchingValue method there is a check to avoid observer nested invocations, will it work correctly?

Yes, it will. This check prevents reentrance in onChanged method of livedata's observers. If an observer in itsonChanged method sets a new value to LiveData it listens to, this new value will be dispatched right after execution of that onChanged method.

Collaborator

svasilinets commented Jun 27, 2017

At the beginning of dispatchingValue method there is a check to avoid observer nested invocations, will it work correctly?

Yes, it will. This check prevents reentrance in onChanged method of livedata's observers. If an observer in itsonChanged method sets a new value to LiveData it listens to, this new value will be dispatched right after execution of that onChanged method.

@sockeqwe

This comment has been minimized.

Show comment
Hide comment
@sockeqwe

sockeqwe Jun 30, 2017

I have worked a little bit on refactoring the SearchViewModel of GithubBrowserSample and would like to hear your feedback.
My forked repo can be found here: https://github.com/sockeqwe/android-architecture-components

So I have introduced a class SearchState. This class basically represents state and can be "rendered" directly by the view. So instead of LiveData<Resource<List<Repo>>> results and LiveData<LoadMoreState> my refactored SearchViewModel only provides a single LiveData<SearchState> state for the view (SearchFragment) to observe since SearchState contains all state information in one single object.

Please note that you could split this class in something like kotlin's sealed class or other solutions. The code itself is far from perfect in my example. It is just a proof of concept.

Whenever the user triggers an action like typing search, scroll to the end to load next page, this action sinks down to the business logic. The business logic knows the current state and applies this action on it to compute the new state (which is a new immutable state object). For "SingleLiveEvents" like displaying a error message in a SnackBar I would like to suggest to apply the same concept. If SnackBar has shown the message the action "clear load more error message" sinks down to the business logic as it would be any other action like a click on a button.

you update a LiveData while you are updating the view because of a modification of the same LiveData .At the beginning of dispatchingValue method there is a check to avoid observer nested invocations, will it work correctly?

@fabioCollini yes, it does. It works as you can see in my linked example repository.

Regarding @JoseAlcerreca questions:

  1. requires more logic in the activity (onDismissed listener and a null check)

Not sure if I would call a listener "more logic". Then each click listener is also more logic? I guess it depends on how you define logic in context of view (like activity etc.). (btw. the number of lines of code is decreased by about 20 lines, not sure how meaningful this information is though).

  1. setLoadMoreErrorShow should be called resetLoadMoreState, or to use the original,setLoadMoreShown, not sure why you added "Error" there.

Yes, naming is not great.

  1. it behaves differently than SingleLiveData, since you're resetting after dismissal while we have a "fire and forget" approach. I understand you want to store state but there could be a case where the app is opened days after the last use and the user sees a misleading error message from 2 days ago. It might not be a good fit for a Snackbar. It is a better fit for a navigation event, for example.

Yes, this are two different approaches: "fire and forget" vs. "single state driven by the business logic".
Sure, this is more a "religious" question and as you have already noticed I see more value in the later approach (which obviously doesn't mean that this approach is the better one) when it comes to predictable state management, debugging and testing. For example, debugging: We can easily log every step we have made from one state to the other state (and which user interaction was involved to trigger the state transition).

One problem with "fire and forget" is that it may leak View implementation details to the underlying layer (like ViewModel, "business logic", or whatever layer you have in your app).
For example: Right now the app is showing a SnackBar if an error has occurred. What If one day we would like to display this error with a traditional TextView? Then we not only have to change code in the view layer, but also in the underlying layers where we have produced the SingleLiveEvent because then it shouldn't be a SingleLiveEvent anymore but for TextView we would use simply MutableLiveData, right?

sockeqwe commented Jun 30, 2017

I have worked a little bit on refactoring the SearchViewModel of GithubBrowserSample and would like to hear your feedback.
My forked repo can be found here: https://github.com/sockeqwe/android-architecture-components

So I have introduced a class SearchState. This class basically represents state and can be "rendered" directly by the view. So instead of LiveData<Resource<List<Repo>>> results and LiveData<LoadMoreState> my refactored SearchViewModel only provides a single LiveData<SearchState> state for the view (SearchFragment) to observe since SearchState contains all state information in one single object.

Please note that you could split this class in something like kotlin's sealed class or other solutions. The code itself is far from perfect in my example. It is just a proof of concept.

Whenever the user triggers an action like typing search, scroll to the end to load next page, this action sinks down to the business logic. The business logic knows the current state and applies this action on it to compute the new state (which is a new immutable state object). For "SingleLiveEvents" like displaying a error message in a SnackBar I would like to suggest to apply the same concept. If SnackBar has shown the message the action "clear load more error message" sinks down to the business logic as it would be any other action like a click on a button.

you update a LiveData while you are updating the view because of a modification of the same LiveData .At the beginning of dispatchingValue method there is a check to avoid observer nested invocations, will it work correctly?

@fabioCollini yes, it does. It works as you can see in my linked example repository.

Regarding @JoseAlcerreca questions:

  1. requires more logic in the activity (onDismissed listener and a null check)

Not sure if I would call a listener "more logic". Then each click listener is also more logic? I guess it depends on how you define logic in context of view (like activity etc.). (btw. the number of lines of code is decreased by about 20 lines, not sure how meaningful this information is though).

  1. setLoadMoreErrorShow should be called resetLoadMoreState, or to use the original,setLoadMoreShown, not sure why you added "Error" there.

Yes, naming is not great.

  1. it behaves differently than SingleLiveData, since you're resetting after dismissal while we have a "fire and forget" approach. I understand you want to store state but there could be a case where the app is opened days after the last use and the user sees a misleading error message from 2 days ago. It might not be a good fit for a Snackbar. It is a better fit for a navigation event, for example.

Yes, this are two different approaches: "fire and forget" vs. "single state driven by the business logic".
Sure, this is more a "religious" question and as you have already noticed I see more value in the later approach (which obviously doesn't mean that this approach is the better one) when it comes to predictable state management, debugging and testing. For example, debugging: We can easily log every step we have made from one state to the other state (and which user interaction was involved to trigger the state transition).

One problem with "fire and forget" is that it may leak View implementation details to the underlying layer (like ViewModel, "business logic", or whatever layer you have in your app).
For example: Right now the app is showing a SnackBar if an error has occurred. What If one day we would like to display this error with a traditional TextView? Then we not only have to change code in the view layer, but also in the underlying layers where we have produced the SingleLiveEvent because then it shouldn't be a SingleLiveEvent anymore but for TextView we would use simply MutableLiveData, right?

@deviant-studio

This comment has been minimized.

Show comment
Hide comment

i've implemented similar approach for ui commands
https://github.com/deviant-studio/energy-meter-scanner/blob/master/app/src/main/java/ds/meterscanner/mvvm/Extensions.kt#L30

and Snackbar example here
https://github.com/deviant-studio/energy-meter-scanner/blob/master/app/src/main/java/ds/meterscanner/mvvm/Command.kt#L8

it would be great to have such functionality out of the box in the upcoming lib release

@JoseAlcerreca

This comment has been minimized.

Show comment
Hide comment
@JoseAlcerreca

JoseAlcerreca Jul 5, 2017

Collaborator

@deviant-studio your Command has two problems:

  • You can't use null as a value
  • If the value is set while an observer is not active, it will be lost. For example, if done from activity.onCreate.
Collaborator

JoseAlcerreca commented Jul 5, 2017

@deviant-studio your Command has two problems:

  • You can't use null as a value
  • If the value is set while an observer is not active, it will be lost. For example, if done from activity.onCreate.
@deviant-studio

This comment has been minimized.

Show comment
Hide comment
@deviant-studio

deviant-studio Jul 5, 2017

@JoseAlcerreca true. that is why i'm going to use your SingleLiveEvent implementation ;)

@JoseAlcerreca true. that is why i'm going to use your SingleLiveEvent implementation ;)

@asantibanez

This comment has been minimized.

Show comment
Hide comment
@asantibanez

asantibanez Jul 19, 2017

Hi! I just started/learning Android Architecture Components and see that how to handle View Actions is a missing piece on the proposed components. By no means I am criticizing the work done by Google. I love what they have brought to the table.

With that being said, I kind of like @deviant-studio approach to the problem. Making all Commands one shot only. Imagine if this approach can be used together with a Event Queue System that the ViewModel would manage. The Events would be like constant ACTIONS that the Activity/Fragment could subscribe to and would be dispatched in order.

A seudo implementation of this would be:
In ViewModel:

dispatch(SHOW_PROGRESS)

//do long work
loadPosts()

dispatch(HIDE_PROGRESS)
dispatch(SHOW_RESULTS)

In Activity:

queueSubscribe {
    switch(event) {
        SHOW_PROGRESS ->  progressBar.setVisibility(View.VISIBLE)
        HIDE_PROGRESS -> progressBar.setVisibility(View.GONE)
        SHOW_RESULTS -> {
            adapter.setPosts(viewModel.posts)
        }
    }
}

Once the event is consumed, it should be dropped from the queue and all should be processed sequentially. In this way, the Activity/Fragment would loose track of all ViewActions that need to be handled by the view.

This is my 2 cents on this. Got the inspiration after working with Vuex (https://vuex.vuejs.org/en/intro.html) on another project (somewhat React-ish) to handle all data flow on one direction.

If this is not good or valid at all, no problem! Just sharing some ideas.

Hi! I just started/learning Android Architecture Components and see that how to handle View Actions is a missing piece on the proposed components. By no means I am criticizing the work done by Google. I love what they have brought to the table.

With that being said, I kind of like @deviant-studio approach to the problem. Making all Commands one shot only. Imagine if this approach can be used together with a Event Queue System that the ViewModel would manage. The Events would be like constant ACTIONS that the Activity/Fragment could subscribe to and would be dispatched in order.

A seudo implementation of this would be:
In ViewModel:

dispatch(SHOW_PROGRESS)

//do long work
loadPosts()

dispatch(HIDE_PROGRESS)
dispatch(SHOW_RESULTS)

In Activity:

queueSubscribe {
    switch(event) {
        SHOW_PROGRESS ->  progressBar.setVisibility(View.VISIBLE)
        HIDE_PROGRESS -> progressBar.setVisibility(View.GONE)
        SHOW_RESULTS -> {
            adapter.setPosts(viewModel.posts)
        }
    }
}

Once the event is consumed, it should be dropped from the queue and all should be processed sequentially. In this way, the Activity/Fragment would loose track of all ViewActions that need to be handled by the view.

This is my 2 cents on this. Got the inspiration after working with Vuex (https://vuex.vuejs.org/en/intro.html) on another project (somewhat React-ish) to handle all data flow on one direction.

If this is not good or valid at all, no problem! Just sharing some ideas.

@gaara87

This comment has been minimized.

Show comment
Hide comment
@gaara87

gaara87 Jul 21, 2017

I love the idea of SingleLiveEvent but i'm thinking, theres 10~ish callbacks, is it better to have n unique SingleLiveEvents or would it be better to have a singleliveevent passing in different event types?

gaara87 commented Jul 21, 2017

I love the idea of SingleLiveEvent but i'm thinking, theres 10~ish callbacks, is it better to have n unique SingleLiveEvents or would it be better to have a singleliveevent passing in different event types?

@etiennelenhart

This comment has been minimized.

Show comment
Hide comment
@etiennelenhart

etiennelenhart Jul 27, 2017

@JoseAlcerreca We tried your SingleLiveEvent implementation and are having problems when using it together with observeForever. removerObserver isn't working correctly since the original Observer is wrapped inside the SingleLiveEvent. Maybe it needs some kind of internal map of wrapped observers?

I created a naive implementation (in Kotlin) which works for us: https://gist.github.com/etiennewelsch/142ab9e080b0144927fd2486cd34feb9

etiennelenhart commented Jul 27, 2017

@JoseAlcerreca We tried your SingleLiveEvent implementation and are having problems when using it together with observeForever. removerObserver isn't working correctly since the original Observer is wrapped inside the SingleLiveEvent. Maybe it needs some kind of internal map of wrapped observers?

I created a naive implementation (in Kotlin) which works for us: https://gist.github.com/etiennewelsch/142ab9e080b0144927fd2486cd34feb9

@jshvarts

This comment has been minimized.

Show comment
Hide comment
@jshvarts

jshvarts Aug 3, 2017

Contributor

@JoseAlcerreca I am successfully using your SingleLiveEvent and vote for including it in the library when released.

Contributor

jshvarts commented Aug 3, 2017

@JoseAlcerreca I am successfully using your SingleLiveEvent and vote for including it in the library when released.

@JoseAlcerreca

This comment has been minimized.

Show comment
Hide comment
@JoseAlcerreca

JoseAlcerreca Aug 3, 2017

Collaborator

@etiennewelsch observeForever is not meant to be used in production (I think!). Even in tests I ended up using observe. If you share more details on how you use it I might be able to help.

Collaborator

JoseAlcerreca commented Aug 3, 2017

@etiennewelsch observeForever is not meant to be used in production (I think!). Even in tests I ended up using observe. If you share more details on how you use it I might be able to help.

@JoseAlcerreca

This comment has been minimized.

Show comment
Hide comment
@JoseAlcerreca

JoseAlcerreca Aug 3, 2017

Collaborator

@gaara87 my suggestion is that you try both approaches, as it doesn't take much time, and decide. In principle 10 callbacks sounds easier to manage than 1.

Edit: But I would like to know your use case, as 10 SingleLiveEvents sounds like a lot!

Collaborator

JoseAlcerreca commented Aug 3, 2017

@gaara87 my suggestion is that you try both approaches, as it doesn't take much time, and decide. In principle 10 callbacks sounds easier to manage than 1.

Edit: But I would like to know your use case, as 10 SingleLiveEvents sounds like a lot!

@etiennelenhart

This comment has been minimized.

Show comment
Hide comment
@etiennelenhart

etiennelenhart Aug 3, 2017

@JoseAlcerreca I was not aware that we shouldn't use observerForever. Good to know, if that's really the case. :)
We're currently doing manual Dependency Injection via a custom Application class and are using the SingleLiveEvents to notify AndroidViewModels of events in our business logic. So there is no LifecycleOwner to pass to observe.

@JoseAlcerreca I was not aware that we shouldn't use observerForever. Good to know, if that's really the case. :)
We're currently doing manual Dependency Injection via a custom Application class and are using the SingleLiveEvents to notify AndroidViewModels of events in our business logic. So there is no LifecycleOwner to pass to observe.

@StefMa

This comment has been minimized.

Show comment
Hide comment
@StefMa

StefMa Aug 4, 2017

This thing is indeed a interesting discussion.
I need a "OneTimeObserver" as well in my project.
I ended up in something like that:

    myLiveData.observeForever(object : Observer<String> {
            override fun onChanged(t: String?) {
                mainModelLiveData.value = MainModel(true, t)
                myLiveData.removeObserver(this)
            }
        })

Now I thinking about to use the SingleEvent class. The problem: I use this in a ViewModel. Which means I don't have a LiveCycle here. Which means. The SingleEvent class can't be used.

Why I listen to LiveData in a ViewModel you think?
I have a similar architecture like described here:
I have a UserRepo which returns a LiveData. But beside of "tunneling" it directly in to the Activtiy/Fragment (within a ViewModel code like that):

fun getUser(): User {
   repo.getUser()
}

I have a similar approve like @sockeqwe.
I have one LiveData object which is accessable from the Activtiy/Fragment which sends one Model outside. Like data class Model(var loading: Boolean, var toolbarTitle: String).
But my Repo can be load multiple user. Which means I have to listen in my ViewModel to the changes of the LiveData from the Repo. If everything loaded (with the "OneTimeObserver") I can send change the Model object and it will be send to the Activtiy/Fragment...

Yeah a lot of text i know 🙃

But: Are there other solutions?
Even if you say observeForever shouldn't be used... Why not? 🤔

StefMa commented Aug 4, 2017

This thing is indeed a interesting discussion.
I need a "OneTimeObserver" as well in my project.
I ended up in something like that:

    myLiveData.observeForever(object : Observer<String> {
            override fun onChanged(t: String?) {
                mainModelLiveData.value = MainModel(true, t)
                myLiveData.removeObserver(this)
            }
        })

Now I thinking about to use the SingleEvent class. The problem: I use this in a ViewModel. Which means I don't have a LiveCycle here. Which means. The SingleEvent class can't be used.

Why I listen to LiveData in a ViewModel you think?
I have a similar architecture like described here:
I have a UserRepo which returns a LiveData. But beside of "tunneling" it directly in to the Activtiy/Fragment (within a ViewModel code like that):

fun getUser(): User {
   repo.getUser()
}

I have a similar approve like @sockeqwe.
I have one LiveData object which is accessable from the Activtiy/Fragment which sends one Model outside. Like data class Model(var loading: Boolean, var toolbarTitle: String).
But my Repo can be load multiple user. Which means I have to listen in my ViewModel to the changes of the LiveData from the Repo. If everything loaded (with the "OneTimeObserver") I can send change the Model object and it will be send to the Activtiy/Fragment...

Yeah a lot of text i know 🙃

But: Are there other solutions?
Even if you say observeForever shouldn't be used... Why not? 🤔

@JoseAlcerreca

This comment has been minimized.

Show comment
Hide comment
@JoseAlcerreca

JoseAlcerreca Aug 4, 2017

Collaborator

@StefMa of course you can use the LiveData in the ViewModel, just observe it from the activity. You can create a getLiveData method in the VM or anobserve(owner, observer). As with any LiveData, the VM exposes it.

Collaborator

JoseAlcerreca commented Aug 4, 2017

@StefMa of course you can use the LiveData in the ViewModel, just observe it from the activity. You can create a getLiveData method in the VM or anobserve(owner, observer). As with any LiveData, the VM exposes it.

@StefMa

This comment has been minimized.

Show comment
Hide comment
@StefMa

StefMa Aug 4, 2017

That's is the point. I don't want to observe it in the activity. As written above. I have multiple LiveData from different "services" in on ViewModel but want only send one "global Modal" to the Activity...

Sent from my Google Nexus 5X using FastHub

StefMa commented Aug 4, 2017

That's is the point. I don't want to observe it in the activity. As written above. I have multiple LiveData from different "services" in on ViewModel but want only send one "global Modal" to the Activity...

Sent from my Google Nexus 5X using FastHub

@JoseAlcerreca

This comment has been minimized.

Show comment
Hide comment
@JoseAlcerreca

JoseAlcerreca Aug 4, 2017

Collaborator

Gotcha. If you're talking with a data source that doesn't need to be scoped to a view, you might want to use observeForever. However, LiveData was designed for the presentation layer, not to replace RX.

Collaborator

JoseAlcerreca commented Aug 4, 2017

Gotcha. If you're talking with a data source that doesn't need to be scoped to a view, you might want to use observeForever. However, LiveData was designed for the presentation layer, not to replace RX.

@etiennelenhart

This comment has been minimized.

Show comment
Hide comment
@etiennelenhart

etiennelenhart Aug 4, 2017

@JoseAlcerreca @StefMa We came to the same conclusion today and just wrote our own Observable implementation which works equally well without the lifecycle overhead. SingleLiveEvent is still really useful for ViewModel to Activity events though.

@JoseAlcerreca @StefMa We came to the same conclusion today and just wrote our own Observable implementation which works equally well without the lifecycle overhead. SingleLiveEvent is still really useful for ViewModel to Activity events though.

@etiennelenhart

This comment has been minimized.

Show comment
Hide comment
@etiennelenhart

etiennelenhart Aug 10, 2017

@JoseAlcerreca We came across a possible bug in the SingleLiveEvent implementation. When multiple observers are registered (e.g. two fragments), only the first is notified since pending is reset immediately. Any thoughts?

@JoseAlcerreca We came across a possible bug in the SingleLiveEvent implementation. When multiple observers are registered (e.g. two fragments), only the first is notified since pending is reset immediately. Any thoughts?

@JoseAlcerreca

This comment has been minimized.

Show comment
Hide comment
@JoseAlcerreca

JoseAlcerreca Aug 10, 2017

Collaborator

Yes, that's working as intended (there's even a warning when you register the second). It's because we can't know how many observers are active (we can only know there are >0) as the LiveData doesn't expose the information.

We created a version that counted the number of active observers but it was too complex for a sample. We'll try to fix it if it's included in the library.

The workaround is to create a SingleLiveEvent per observer.

Collaborator

JoseAlcerreca commented Aug 10, 2017

Yes, that's working as intended (there's even a warning when you register the second). It's because we can't know how many observers are active (we can only know there are >0) as the LiveData doesn't expose the information.

We created a version that counted the number of active observers but it was too complex for a sample. We'll try to fix it if it's included in the library.

The workaround is to create a SingleLiveEvent per observer.

@etiennelenhart

This comment has been minimized.

Show comment
Hide comment
@etiennelenhart

etiennelenhart Aug 11, 2017

@JoseAlcerreca Ah, sorry I totally forgot that you put in that warning. It seems to be quite difficult to get ViewModel to Activity communication right. I'm pretty confident, that you'll come up with something though. The Components already simplified our projects a lot.

@JoseAlcerreca Ah, sorry I totally forgot that you put in that warning. It seems to be quite difficult to get ViewModel to Activity communication right. I'm pretty confident, that you'll come up with something though. The Components already simplified our projects a lot.

@mynote

This comment has been minimized.

Show comment
Hide comment
@mynote

mynote Aug 21, 2017

Even if you can pass all your "view"-actions into your hosting activity, you could use your view operations in the viewmodel without "context" leaks or similar.

Sample:

public class MyViewModel extends ViewModel {
    @Nullable
    private WeakReference<NavigationController> navigationController;

    public void onWhateverAction() {
            if (navigationController != null && navigationController.get() != null) 
                     navigationController.get().navigateToWhatever();
    }
  }

Do not forget to set the navigationController in a view (for example activity) and share the viewmodel around.

mynote commented Aug 21, 2017

Even if you can pass all your "view"-actions into your hosting activity, you could use your view operations in the viewmodel without "context" leaks or similar.

Sample:

public class MyViewModel extends ViewModel {
    @Nullable
    private WeakReference<NavigationController> navigationController;

    public void onWhateverAction() {
            if (navigationController != null && navigationController.get() != null) 
                     navigationController.get().navigateToWhatever();
    }
  }

Do not forget to set the navigationController in a view (for example activity) and share the viewmodel around.

@petrnohejl

This comment has been minimized.

Show comment
Hide comment
@petrnohejl

petrnohejl Aug 30, 2017

Hello everybody! This is an interesting discussion. I like the solution with SingleLiveEvent from @JoseAlcerreca. I use it in my playground project and it works pretty well.

This is my requirements for handling view actions (VM -> V communication):

  • Event-driven communication (fire and forget, not stateful)
  • No interface (interface segregation)
  • Using LiveData API
  • Run action only if context is available
  • Run action only if activity is in resumed state
  • Definition of UI action in the view layer
  • Pass any object as an argument(s)
  • Support multiple event types

Since I usually have more than one live event in my view models (e.g. toast event, snackbar event, dialog event, start a new activity event etc.), I came up with a LiveBus which is basically a map with SingleLiveEvent instances for each event type.

So to answer @gaara87 question: I have n unique SingleLiveEvent objects stored in the LiveBus to keep it simpler. I don't have to define n SingleLiveEvent objects in my view model, but just one LiveBus object. New instance of SingleLiveEvent is created automatically inside the bus once the specific event is observed or sent for a first time.

See this example how I use it:

public class ExampleFragment extends LifecycleFragment {

	...

	private void setupObservers() {
		getViewModel().liveBus.observe(this, ToastEvent.class, this::showToast);
		getViewModel().liveBus.observe(this, SnackbarEvent.class, this::showSnackbar);
	}

	private void showToast(ToastEvent event) {
		Toast.makeText(getActivity(), event.message, Toast.LENGTH_LONG).show();
	}

	private void showSnackbar(SnackbarEvent event) {
		Snackbar.make(getView(), event.message, Snackbar.LENGTH_LONG).show();
	}
}
public class ExampleViewModel extends AndroidViewModel {
	public final LiveBus liveBus = new LiveBus();

	...

	private void handleError(String message) {
		liveBus.send(new ToastEvent(message));
	}
}

You can check my Stocks playground project to see the whole code. Please note that in my code I renamed SingleLiveEvent to LiveEvent.

What do you guys think about this solution?

petrnohejl commented Aug 30, 2017

Hello everybody! This is an interesting discussion. I like the solution with SingleLiveEvent from @JoseAlcerreca. I use it in my playground project and it works pretty well.

This is my requirements for handling view actions (VM -> V communication):

  • Event-driven communication (fire and forget, not stateful)
  • No interface (interface segregation)
  • Using LiveData API
  • Run action only if context is available
  • Run action only if activity is in resumed state
  • Definition of UI action in the view layer
  • Pass any object as an argument(s)
  • Support multiple event types

Since I usually have more than one live event in my view models (e.g. toast event, snackbar event, dialog event, start a new activity event etc.), I came up with a LiveBus which is basically a map with SingleLiveEvent instances for each event type.

So to answer @gaara87 question: I have n unique SingleLiveEvent objects stored in the LiveBus to keep it simpler. I don't have to define n SingleLiveEvent objects in my view model, but just one LiveBus object. New instance of SingleLiveEvent is created automatically inside the bus once the specific event is observed or sent for a first time.

See this example how I use it:

public class ExampleFragment extends LifecycleFragment {

	...

	private void setupObservers() {
		getViewModel().liveBus.observe(this, ToastEvent.class, this::showToast);
		getViewModel().liveBus.observe(this, SnackbarEvent.class, this::showSnackbar);
	}

	private void showToast(ToastEvent event) {
		Toast.makeText(getActivity(), event.message, Toast.LENGTH_LONG).show();
	}

	private void showSnackbar(SnackbarEvent event) {
		Snackbar.make(getView(), event.message, Snackbar.LENGTH_LONG).show();
	}
}
public class ExampleViewModel extends AndroidViewModel {
	public final LiveBus liveBus = new LiveBus();

	...

	private void handleError(String message) {
		liveBus.send(new ToastEvent(message));
	}
}

You can check my Stocks playground project to see the whole code. Please note that in my code I renamed SingleLiveEvent to LiveEvent.

What do you guys think about this solution?

@hartmannj

This comment has been minimized.

Show comment
Hide comment
@hartmannj

hartmannj Sep 7, 2017

I had the same problem with SingleLiveEvent only triggering the top most observer and not respecting the removeObserver function. As mentioned already, this could potentially lead to unexpected behaviour if the ViewModel is used in several classes.
So I came up with another solution where all registered observers are getting called only once! I guess complexity can be reduced if the number of active observers are exposed in a future library release.

public class OneShotLiveEvent<T> extends MutableLiveData<T> {

    private final ConcurrentHashMap<Observer<T>, Pair<Observer<T>, AtomicBoolean>> mPendingObservers = new ConcurrentHashMap<>();

    @MainThread
    public void observe(LifecycleOwner owner, Observer<T> observer) {

        Observer<T> interceptor = new Observer<T>() {
            @Override
            public void onChanged(@Nullable T t) {
                Observer<T> observerToTrigger = null;
                synchronized (mPendingObservers) {
                    if(mPendingObservers.containsKey(this) && mPendingObservers.get(this).second.compareAndSet(true, false)) {
                        observerToTrigger = mPendingObservers.get(this).first;
                    }
                }
                if(observerToTrigger != null) {
                    observerToTrigger.onChanged(t);
                }
            }
        };

        synchronized (mPendingObservers) {
            mPendingObservers.put(interceptor, Pair.create(observer, new AtomicBoolean(false)));
        }

        // Observe the internal MutableLiveData
        super.observe(owner, interceptor);
    }

    @Override
    public void removeObserver(Observer<T> observer) {
        super.removeObserver(observer);
        synchronized (mPendingObservers) {
            for(Map.Entry<Observer<T>, Pair<Observer<T>, AtomicBoolean>> entry : mPendingObservers.entrySet()) {
                if(observer == entry.getKey() || observer == entry.getValue().first) {
                    mPendingObservers.remove(entry.getKey());
                    break;
                }
            }
        }
    }

    @MainThread
    public void setValue(@Nullable T t) {
        synchronized (mPendingObservers) {
            for(Pair<Observer<T>, AtomicBoolean> pending : mPendingObservers.values()) {
                pending.second.set(true);
            }
        }
        super.setValue(t);
    }

    /**
     * Used for cases where T is Void, to make calls cleaner.
     */
    @MainThread
    public void call() {
        setValue(null);
    }
}

What do you guys think?

Regarding one generic single live event for all events vs one for each event, I am tending to prefer the latter simply because in some cases you might want to add result data to it without doing type castings.

hartmannj commented Sep 7, 2017

I had the same problem with SingleLiveEvent only triggering the top most observer and not respecting the removeObserver function. As mentioned already, this could potentially lead to unexpected behaviour if the ViewModel is used in several classes.
So I came up with another solution where all registered observers are getting called only once! I guess complexity can be reduced if the number of active observers are exposed in a future library release.

public class OneShotLiveEvent<T> extends MutableLiveData<T> {

    private final ConcurrentHashMap<Observer<T>, Pair<Observer<T>, AtomicBoolean>> mPendingObservers = new ConcurrentHashMap<>();

    @MainThread
    public void observe(LifecycleOwner owner, Observer<T> observer) {

        Observer<T> interceptor = new Observer<T>() {
            @Override
            public void onChanged(@Nullable T t) {
                Observer<T> observerToTrigger = null;
                synchronized (mPendingObservers) {
                    if(mPendingObservers.containsKey(this) && mPendingObservers.get(this).second.compareAndSet(true, false)) {
                        observerToTrigger = mPendingObservers.get(this).first;
                    }
                }
                if(observerToTrigger != null) {
                    observerToTrigger.onChanged(t);
                }
            }
        };

        synchronized (mPendingObservers) {
            mPendingObservers.put(interceptor, Pair.create(observer, new AtomicBoolean(false)));
        }

        // Observe the internal MutableLiveData
        super.observe(owner, interceptor);
    }

    @Override
    public void removeObserver(Observer<T> observer) {
        super.removeObserver(observer);
        synchronized (mPendingObservers) {
            for(Map.Entry<Observer<T>, Pair<Observer<T>, AtomicBoolean>> entry : mPendingObservers.entrySet()) {
                if(observer == entry.getKey() || observer == entry.getValue().first) {
                    mPendingObservers.remove(entry.getKey());
                    break;
                }
            }
        }
    }

    @MainThread
    public void setValue(@Nullable T t) {
        synchronized (mPendingObservers) {
            for(Pair<Observer<T>, AtomicBoolean> pending : mPendingObservers.values()) {
                pending.second.set(true);
            }
        }
        super.setValue(t);
    }

    /**
     * Used for cases where T is Void, to make calls cleaner.
     */
    @MainThread
    public void call() {
        setValue(null);
    }
}

What do you guys think?

Regarding one generic single live event for all events vs one for each event, I am tending to prefer the latter simply because in some cases you might want to add result data to it without doing type castings.

@sandys

This comment has been minimized.

Show comment
Hide comment
@sandys

sandys Sep 13, 2017

Is anyone using something like EventBus to trigger navigation events ? After reading this thread, I'm kind of confused on what Google's official recommendations here are.
Usually, these things work like CQRS or the Facebook Flux model - where the view observes the state manager (the viewmodel?) and then if state changes, the view renders itself. But in the ViewModel architecture, the statemanager is explicitly calling functions... so I'm not very sure.

sandys commented Sep 13, 2017

Is anyone using something like EventBus to trigger navigation events ? After reading this thread, I'm kind of confused on what Google's official recommendations here are.
Usually, these things work like CQRS or the Facebook Flux model - where the view observes the state manager (the viewmodel?) and then if state changes, the view renders itself. But in the ViewModel architecture, the statemanager is explicitly calling functions... so I'm not very sure.

@asantibanez

This comment has been minimized.

Show comment
Hide comment
@asantibanez

asantibanez Sep 13, 2017

Hey Sandis. I am also on the same boat not knowing what pattern to use or which one is recommended. That said, I have been using an event bus for this kind of communication and it has been working good. Hope it helps.

Hey Sandis. I am also on the same boat not knowing what pattern to use or which one is recommended. That said, I have been using an event bus for this kind of communication and it has been working good. Hope it helps.

@Daio-io

This comment has been minimized.

Show comment
Hide comment
@Daio-io

Daio-io Nov 23, 2017

Taking the the SingleLiveEvent idea. Another approach I have thought about taking is using nulls to prevent events. If you want to just fire a single value that does not require holding state (often true for a single action) then you can create something like below (ActionLiveData.kt) that automatically nulls out the value after it has sent the change. Then if the value is null just return early without notifying the observer leaving you open to send the next action. This all depends on whether you need null values, but it is another approach if anyone is looking for a solution.

The code probably explains it better:

class ActionLiveData<T> : MutableLiveData<T>() {

    @MainThread
    override fun observe(owner: LifecycleOwner, observer: Observer<T?>) {

       // Having more than one observer is obviously a per case basis, so just modify accordingly.
        if (hasObservers()) {
            throw Throwable("Only one observer at a time may subscribe to a SingleActionLiveData")
        }

        super.observe(owner, Observer { data ->
            if (data == null)  return
            observer.onChanged(data)
            value = null
        })
    }

    // Just a nicely named method that wraps setting the value
    @MainThread
    fun sendAction(data: T) {
        value = data
    }
}

Daio-io commented Nov 23, 2017

Taking the the SingleLiveEvent idea. Another approach I have thought about taking is using nulls to prevent events. If you want to just fire a single value that does not require holding state (often true for a single action) then you can create something like below (ActionLiveData.kt) that automatically nulls out the value after it has sent the change. Then if the value is null just return early without notifying the observer leaving you open to send the next action. This all depends on whether you need null values, but it is another approach if anyone is looking for a solution.

The code probably explains it better:

class ActionLiveData<T> : MutableLiveData<T>() {

    @MainThread
    override fun observe(owner: LifecycleOwner, observer: Observer<T?>) {

       // Having more than one observer is obviously a per case basis, so just modify accordingly.
        if (hasObservers()) {
            throw Throwable("Only one observer at a time may subscribe to a SingleActionLiveData")
        }

        super.observe(owner, Observer { data ->
            if (data == null)  return
            observer.onChanged(data)
            value = null
        })
    }

    // Just a nicely named method that wraps setting the value
    @MainThread
    fun sendAction(data: T) {
        value = data
    }
}
@ebrowne72

This comment has been minimized.

Show comment
Hide comment
@ebrowne72

ebrowne72 Nov 25, 2017

Some good ideas here, which gave me my own idea: use Kotlin function literals with receiver to make the event work more like a method call. I wrote up something about it here.

Some good ideas here, which gave me my own idea: use Kotlin function literals with receiver to make the event work more like a method call. I wrote up something about it here.

@ilians

This comment has been minimized.

Show comment
Hide comment
@ilians

ilians Jan 15, 2018

Below is a slight modification of the SingleLiveEvent which allows the view to consume the onChanged events if desired. As mentioned by @sockeqwe, the original SingleLiveEvent class leaks implementation details.

Right now the app is showing a SnackBar if an error has occurred. What If one day we would like to display this error with a traditional TextView? Then we not only have to change code in the view layer, but also in the underlying layers where we have produced the SingleLiveEvent because then it shouldn't be a SingleLiveEvent anymore but for TextView we would use simply MutableLiveData, right?

By using the ConsumableLiveData the view decides the implementation by either observing using the regular observe method or the observeAndConsume method. You'll be exposing a ConsumableLiveData object instead of LiveData so the view model is unaware of the actual implementation.

open class ConsumableLiveData<T> : LiveData<T>() {

    private val mConsumed = AtomicBoolean(false)

    @MainThread
    fun observeAndConsume(owner: LifecycleOwner, observer: Observer<T>) {

        if (hasActiveObservers()) {
            Timber.w("Multiple observers registered but only one will be notified of changes.")
        }

        // Observe the internal MutableLiveData
        super.observe(owner, Observer { t ->
            if (mConsumed.compareAndSet(false, true)) {
                observer.onChanged(t)
            }
        })
    }

    @MainThread
    override fun setValue(t: T?) {
        mConsumed.set(false)
        super.setValue(t)
    }
}
/**
 * The mutable implementation of the [ConsumableLiveData]
 */
class MutableConsumableLiveData<T> : ConsumableLiveData<T>() {

    /**
     * Override to expose method (protected to public)
     */
    @MainThread
    public override fun setValue(t: T?) {
        super.setValue(t)
    }
}

ilians commented Jan 15, 2018

Below is a slight modification of the SingleLiveEvent which allows the view to consume the onChanged events if desired. As mentioned by @sockeqwe, the original SingleLiveEvent class leaks implementation details.

Right now the app is showing a SnackBar if an error has occurred. What If one day we would like to display this error with a traditional TextView? Then we not only have to change code in the view layer, but also in the underlying layers where we have produced the SingleLiveEvent because then it shouldn't be a SingleLiveEvent anymore but for TextView we would use simply MutableLiveData, right?

By using the ConsumableLiveData the view decides the implementation by either observing using the regular observe method or the observeAndConsume method. You'll be exposing a ConsumableLiveData object instead of LiveData so the view model is unaware of the actual implementation.

open class ConsumableLiveData<T> : LiveData<T>() {

    private val mConsumed = AtomicBoolean(false)

    @MainThread
    fun observeAndConsume(owner: LifecycleOwner, observer: Observer<T>) {

        if (hasActiveObservers()) {
            Timber.w("Multiple observers registered but only one will be notified of changes.")
        }

        // Observe the internal MutableLiveData
        super.observe(owner, Observer { t ->
            if (mConsumed.compareAndSet(false, true)) {
                observer.onChanged(t)
            }
        })
    }

    @MainThread
    override fun setValue(t: T?) {
        mConsumed.set(false)
        super.setValue(t)
    }
}
/**
 * The mutable implementation of the [ConsumableLiveData]
 */
class MutableConsumableLiveData<T> : ConsumableLiveData<T>() {

    /**
     * Override to expose method (protected to public)
     */
    @MainThread
    public override fun setValue(t: T?) {
        super.setValue(t)
    }
}

@gunhansancar

This comment has been minimized.

Show comment
Hide comment
@gunhansancar

gunhansancar Feb 1, 2018

@ilians To me your solution is exactly the same as SingleLiveEvent.

So appearently for this single events there are 2 possible ways:
1- Using SingleLiveEvent
2- Producing second event/action/state after the first is consumed like Action.Error, Action.ErrorShown

Are there any other solutions?
Will the SingleLiveEvent be the recommended solution by the Architecture Components Guidelines ?

@ilians To me your solution is exactly the same as SingleLiveEvent.

So appearently for this single events there are 2 possible ways:
1- Using SingleLiveEvent
2- Producing second event/action/state after the first is consumed like Action.Error, Action.ErrorShown

Are there any other solutions?
Will the SingleLiveEvent be the recommended solution by the Architecture Components Guidelines ?

@JoseAlcerreca

This comment has been minimized.

Show comment
Hide comment
@JoseAlcerreca

JoseAlcerreca Feb 1, 2018

Collaborator

Those are basically the solutions. For a big project, I recommend (2). If you know what you're doing, (1).

SingleLiveEvent won't be the recommended way, because it can be buggy.

We'll offer some other solution. I'm pushing for this every day! 🙏

Collaborator

JoseAlcerreca commented Feb 1, 2018

Those are basically the solutions. For a big project, I recommend (2). If you know what you're doing, (1).

SingleLiveEvent won't be the recommended way, because it can be buggy.

We'll offer some other solution. I'm pushing for this every day! 🙏

@mhsmith

This comment has been minimized.

Show comment
Hide comment
@mhsmith

mhsmith Mar 6, 2018

@fabioCollini, @asantibanez, and anyone else who's interested in observing an ordered sequence of events without losing those which happen when inactive:

I've developed a general solution to this: BufferedLiveEvent (see also its unit tests). It's a subclass of SingleLiveEvent, which I've also modified to correctly implement removeObserver, and to prevent multiple observers being registered.

mhsmith commented Mar 6, 2018

@fabioCollini, @asantibanez, and anyone else who's interested in observing an ordered sequence of events without losing those which happen when inactive:

I've developed a general solution to this: BufferedLiveEvent (see also its unit tests). It's a subclass of SingleLiveEvent, which I've also modified to correctly implement removeObserver, and to prevent multiple observers being registered.

@etiennelenhart

This comment has been minimized.

Show comment
Hide comment
@etiennelenhart

etiennelenhart Mar 12, 2018

I continuously refined the idea of the ViewModel UI interaction and came up with a complete library to handle immutable view states on top of of Architecture Components. If anyone is interested, it's available on GitHub: https://github.com/etiennelenhart/Eiffel
It packs some other goodies but is really light-weight. The interesting part is tackled in the ViewEvent section.
Any feedback is more than welcome.

I continuously refined the idea of the ViewModel UI interaction and came up with a complete library to handle immutable view states on top of of Architecture Components. If anyone is interested, it's available on GitHub: https://github.com/etiennelenhart/Eiffel
It packs some other goodies but is really light-weight. The interesting part is tackled in the ViewEvent section.
Any feedback is more than welcome.

@JoseAlcerreca

This comment has been minimized.

Show comment
Hide comment
@vitorvigano

This comment has been minimized.

Show comment
Hide comment
@vitorvigano

vitorvigano May 21, 2018

What about this, using Kotlin’s function literals with receiver feature?

https://medium.com/@erik_90880/sending-events-from-an-mvvm-view-model-with-kotlin-19fdce61dcb9

vitorvigano commented May 21, 2018

What about this, using Kotlin’s function literals with receiver feature?

https://medium.com/@erik_90880/sending-events-from-an-mvvm-view-model-with-kotlin-19fdce61dcb9

@baoti

This comment has been minimized.

Show comment
Hide comment
@baoti

baoti May 21, 2018

@vitorvigano The function keep a hidden reference of Activity/Fragment, When and How to release it?

baoti commented May 21, 2018

@vitorvigano The function keep a hidden reference of Activity/Fragment, When and How to release it?

@ebrowne72

This comment has been minimized.

Show comment
Hide comment
@ebrowne72

ebrowne72 May 21, 2018

@baoti Actually it keeps a very explicit reference to the Activity/Fragment. But since that reference is held by a LiveData object, when the Activity/Fragment's lifecycle goes to the DESTROYED state the LiveData will remove the observer.

@baoti Actually it keeps a very explicit reference to the Activity/Fragment. But since that reference is held by a LiveData object, when the Activity/Fragment's lifecycle goes to the DESTROYED state the LiveData will remove the observer.

@baoti

This comment has been minimized.

Show comment
Hide comment
@baoti

baoti May 22, 2018

@ebrowne72 No, the LiveData may be hold by ViewModel, and the reference is not hold by LiveData's observer, it's hold by LiveData's value. The value can be hold a long time which it cross many screen rotate operations. The Activity/Fragment reference is leaked.

Deleted, it is wrong.

baoti commented May 22, 2018

@ebrowne72 No, the LiveData may be hold by ViewModel, and the reference is not hold by LiveData's observer, it's hold by LiveData's value. The value can be hold a long time which it cross many screen rotate operations. The Activity/Fragment reference is leaked.

Deleted, it is wrong.

@baoti

This comment has been minimized.

Show comment
Hide comment
@baoti

baoti May 22, 2018

@ebrowne72 Yes, you're right. I made some mistakes. The eventHandler is held by a LiveData's observer, and it keeps the reference of the Activity/Fragment, not the event. The event doesn't hold any reference of the Activity/Fragment.

baoti commented May 22, 2018

@ebrowne72 Yes, you're right. I made some mistakes. The eventHandler is held by a LiveData's observer, and it keeps the reference of the Activity/Fragment, not the event. The event doesn't hold any reference of the Activity/Fragment.

@avegrv

This comment has been minimized.

Show comment
Hide comment
@avegrv

avegrv May 30, 2018

I think the solution of SingleLiveEvent is not accurate and does not cover most cases. For example, if we use RxJava for download some data from Repository, we should send minimum 3 events to View: ShowProgress, HideProgress and ShowContent. To implement this with SingleLiveEvent, we need use 3 SingleLiveEvents. As the consequence the number of events growths dramatically.

I suggest the solution that is based on queue of commands:

  • the ViewModel adds commands to queue.
  • the View subscribes with Subscriber on changes of queue.
  • the Subscriber retrieves commands from the head of queue and executes them.

The described algorithm is presented in the following code:

  • LiveData with queue of commands:
class CommandsLiveData<T> : MutableLiveData<ConcurrentLinkedQueue<T>>() {
    fun onNext(value: T) {
        var commands = getValue()
        if (commands == null) {
            commands = ConcurrentLinkedQueue<T>()
        }
        commands.add(value)
        setValue(commands)
    }
}
  • Observer:
inline fun <reified T : Any, reified L : CommandsLiveData<T>> LifecycleOwner.observe(liveData: L, noinline block: (T) -> Unit) {
    liveData.observe(this, Observer<ConcurrentLinkedQueue<T>> { commands ->
        if (commands == null) {
            return@Observer
        }
        var command: T?
        do {
            command = commands.poll()
            if (command != null) {
                block.invoke(command)
            }
        } while (command != null)
    })
  • usage:
// In ViewModel
useCase.loadInfo()
        .doOnSubscribe { liveData.onNext(ShowProgress()) }
        .doAfterTerminate { liveData.onNext(HideProgress()) }
        .subscribe(
                  ::handleSuccess,
                  ::handleError
         )

// In View
viewModel = getViewModel()
observe(viewModel.liveData, ::processModel)

private fun processModel(model: Model) {
        when(model) {
            is ShowProgress -> showProgress()
            is HideProgress -> hideProgress()
            is Content -> showContent()
        }
}

// Model
sealed class Model {
    class ShowProgress : Model()
    class HideProgress : Model()
    class Content(): Model()
}

avegrv commented May 30, 2018

I think the solution of SingleLiveEvent is not accurate and does not cover most cases. For example, if we use RxJava for download some data from Repository, we should send minimum 3 events to View: ShowProgress, HideProgress and ShowContent. To implement this with SingleLiveEvent, we need use 3 SingleLiveEvents. As the consequence the number of events growths dramatically.

I suggest the solution that is based on queue of commands:

  • the ViewModel adds commands to queue.
  • the View subscribes with Subscriber on changes of queue.
  • the Subscriber retrieves commands from the head of queue and executes them.

The described algorithm is presented in the following code:

  • LiveData with queue of commands:
class CommandsLiveData<T> : MutableLiveData<ConcurrentLinkedQueue<T>>() {
    fun onNext(value: T) {
        var commands = getValue()
        if (commands == null) {
            commands = ConcurrentLinkedQueue<T>()
        }
        commands.add(value)
        setValue(commands)
    }
}
  • Observer:
inline fun <reified T : Any, reified L : CommandsLiveData<T>> LifecycleOwner.observe(liveData: L, noinline block: (T) -> Unit) {
    liveData.observe(this, Observer<ConcurrentLinkedQueue<T>> { commands ->
        if (commands == null) {
            return@Observer
        }
        var command: T?
        do {
            command = commands.poll()
            if (command != null) {
                block.invoke(command)
            }
        } while (command != null)
    })
  • usage:
// In ViewModel
useCase.loadInfo()
        .doOnSubscribe { liveData.onNext(ShowProgress()) }
        .doAfterTerminate { liveData.onNext(HideProgress()) }
        .subscribe(
                  ::handleSuccess,
                  ::handleError
         )

// In View
viewModel = getViewModel()
observe(viewModel.liveData, ::processModel)

private fun processModel(model: Model) {
        when(model) {
            is ShowProgress -> showProgress()
            is HideProgress -> hideProgress()
            is Content -> showContent()
        }
}

// Model
sealed class Model {
    class ShowProgress : Model()
    class HideProgress : Model()
    class Content(): Model()
}
@ebrowne72

This comment has been minimized.

Show comment
Hide comment
@ebrowne72

ebrowne72 May 30, 2018

@avegrv I don't think this queue of commands works very well. What if the View receives the ShowProgress command, but while the data is downloading the View is destroyed and recreated? The ShowProgress command has been removed from the queue, so the View doesn't know what to do.

The View Model should hold the state, and the View should render the state. In this case I'd use a sealed Model class with two subclasses: InProgress and Content. Then use a regular LiveData. If the LiveData's data is an InProgress object, then show the progress bar. If it's a Content object, then hide the progress bar and show the data.

@avegrv I don't think this queue of commands works very well. What if the View receives the ShowProgress command, but while the data is downloading the View is destroyed and recreated? The ShowProgress command has been removed from the queue, so the View doesn't know what to do.

The View Model should hold the state, and the View should render the state. In this case I'd use a sealed Model class with two subclasses: InProgress and Content. Then use a regular LiveData. If the LiveData's data is an InProgress object, then show the progress bar. If it's a Content object, then hide the progress bar and show the data.

@avegrv

This comment has been minimized.

Show comment
Hide comment
@avegrv

avegrv May 31, 2018

@ebrowne72 My solution works well, when the view stores viewState by it self. For example, when View show/hide dialogs when receives commands ShowProgress/HideProgress.

avegrv commented May 31, 2018

@ebrowne72 My solution works well, when the view stores viewState by it self. For example, when View show/hide dialogs when receives commands ShowProgress/HideProgress.

@gunhansancar

This comment has been minimized.

Show comment
Hide comment
@gunhansancar

gunhansancar May 31, 2018

@avegrv Let's not complicate things here. The purpose of single live event is for something different. Showing, hiding etc. is something continues. So you're not supposed to have 3 different events stream for that. You gotta have a state representation of your screen and push the changes through normal livedata stream. For one time(single) events like changing the activity, showing toast message there is the need for SingleLiveEvent stuff.

However recently @JoseAlcerreca shared a concept about consumable events which serve in that case better (please read his article for further details).

And for your last comment, please don't store any kind of state on the view level. It defeats the purpose of everything here. Let's keep our Views as dummy as possible and introduce ViewModel or Presenter for the states.

Cheers.

@avegrv Let's not complicate things here. The purpose of single live event is for something different. Showing, hiding etc. is something continues. So you're not supposed to have 3 different events stream for that. You gotta have a state representation of your screen and push the changes through normal livedata stream. For one time(single) events like changing the activity, showing toast message there is the need for SingleLiveEvent stuff.

However recently @JoseAlcerreca shared a concept about consumable events which serve in that case better (please read his article for further details).

And for your last comment, please don't store any kind of state on the view level. It defeats the purpose of everything here. Let's keep our Views as dummy as possible and introduce ViewModel or Presenter for the states.

Cheers.

@avegrv

This comment has been minimized.

Show comment
Hide comment
@avegrv

avegrv May 31, 2018

@gunhansancar thanks for your comment. It seems that I confused all by my sample. The aim of CommandsLiveData was in reusing of SingleLiveEvent. Because for 3-4 events I need 3-4 SingleLiveEvent - is not as good as it should be.

avegrv commented May 31, 2018

@gunhansancar thanks for your comment. It seems that I confused all by my sample. The aim of CommandsLiveData was in reusing of SingleLiveEvent. Because for 3-4 events I need 3-4 SingleLiveEvent - is not as good as it should be.

@mbircu-ellation

This comment has been minimized.

Show comment
Hide comment
@mbircu-ellation

mbircu-ellation Jun 20, 2018

This class should cover almost all usecases.

class MyLiveData<T> : MutableLiveData<T>() {
    private var lastHandledItem: T? = null

    fun observeSingleEvent(owner: LifecycleOwner, observer: (T?) -> Unit) {
        this.observe(owner, Observer {
            if (it != lastHandledItem) {
                observer(it)
                lastHandledItem = it
            }
        })
    }
}

Works as regular live data. You can subscribe using 2 methods:
observe -> will notify every time
observeSingleEvent -> will notify for each new event

This class should cover almost all usecases.

class MyLiveData<T> : MutableLiveData<T>() {
    private var lastHandledItem: T? = null

    fun observeSingleEvent(owner: LifecycleOwner, observer: (T?) -> Unit) {
        this.observe(owner, Observer {
            if (it != lastHandledItem) {
                observer(it)
                lastHandledItem = it
            }
        })
    }
}

Works as regular live data. You can subscribe using 2 methods:
observe -> will notify every time
observeSingleEvent -> will notify for each new event

@ussernamenikita

This comment has been minimized.

Show comment
Hide comment
@ussernamenikita

ussernamenikita Jun 28, 2018

@mbircu-ellation But it will be work only for one observer

private static class ObserverWrapper<T> implements Observer<T> {
    private T dataLink = null;

    @NonNull
    private Observer<T> original;

    ObserverWrapper(@NonNull Observer<T> original) {
        this.original = original;
    }

    @Override
    public void onChanged(@Nullable T t) {
        if( !equals(dataLink,t) ){
            original.onChanged(t);
        }
        dataLink = t;
    }

    private boolean equals(T dataLink, T t) {
        if(dataLink == null && t == null){
            return true;
        } else if(dataLink != null){
            return dataLink.equals(t);
        }else{
            return t.equals(dataLink);
        }
    }
}

This wrapper will be work for all observers, only for new values.

ussernamenikita commented Jun 28, 2018

@mbircu-ellation But it will be work only for one observer

private static class ObserverWrapper<T> implements Observer<T> {
    private T dataLink = null;

    @NonNull
    private Observer<T> original;

    ObserverWrapper(@NonNull Observer<T> original) {
        this.original = original;
    }

    @Override
    public void onChanged(@Nullable T t) {
        if( !equals(dataLink,t) ){
            original.onChanged(t);
        }
        dataLink = t;
    }

    private boolean equals(T dataLink, T t) {
        if(dataLink == null && t == null){
            return true;
        } else if(dataLink != null){
            return dataLink.equals(t);
        }else{
            return t.equals(dataLink);
        }
    }
}

This wrapper will be work for all observers, only for new values.

@guelo

This comment has been minimized.

Show comment
Hide comment
@guelo

guelo Jul 10, 2018

Here's an example using RxJava instead of LiveData. First, for the single-shot style, similar to SingleLiveEvent. The trick is to use a PublishSubject which means that a second subscriber won't get the previous event:

class MyViewModel : ViewModel() {
    private val showSnackbarPublisher = PublishSubject.create<String>()
    val showSnackbarObservable: Observable<String> = showSnackbarPublisher

    private fun doStuff() {
        showSnackbarPublisher.onNext("show this message")
    }
}

class MyActivity : AppCompatActivity() {
    private fun subscribe() {
        vm.showSnackbarObservable
                .subscribe {message ->
                    Snackbar.make(rootView, message, Snackbar.LENGTH_LONG)
                            .show()
                }
    }

    val vm = MyViewModel()
}

One issue with this technique is that if the ViewModel sends the event before the View is subscribed then it will never get it.

Here's an example using the round-trip style, similar to what @sockeqwe recommends. The trick here is to use a BehaviorSubject so the event will be there waiting for any subscriber.

class MyViewModel : ViewModel() {

    sealed class SnackbarState {
        data class Show(val message: String) : SnackbarState()
        object DontShow : SnackbarState()
    }

    private val showSnackbarPublisher = BehaviorSubject.create<SnackbarState>()
    val showSnackbarObservable: Observable<SnackbarState> = showSnackbarPublisher

    private fun doStuff() {
        showSnackbarPublisher.onNext(SnackbarState.Show("show this message"))
    }

    fun onSnackbarShown() {
        showSnackbarPublisher.onNext(SnackbarState.DontShow)
    }
}

class MyActivity : AppCompatActivity() {

    private fun subscribe() {
        vm.showSnackbarObservable
                .subscribe {
                    if (it is MyViewModel.SnackbarState.Show) {
                        Snackbar.make(rootView, it.message, Snackbar.LENGTH_LONG)
                                .show()
                        
                        vm.onSnackbarShown()
                    }
                }
    }
}

guelo commented Jul 10, 2018

Here's an example using RxJava instead of LiveData. First, for the single-shot style, similar to SingleLiveEvent. The trick is to use a PublishSubject which means that a second subscriber won't get the previous event:

class MyViewModel : ViewModel() {
    private val showSnackbarPublisher = PublishSubject.create<String>()
    val showSnackbarObservable: Observable<String> = showSnackbarPublisher

    private fun doStuff() {
        showSnackbarPublisher.onNext("show this message")
    }
}

class MyActivity : AppCompatActivity() {
    private fun subscribe() {
        vm.showSnackbarObservable
                .subscribe {message ->
                    Snackbar.make(rootView, message, Snackbar.LENGTH_LONG)
                            .show()
                }
    }

    val vm = MyViewModel()
}

One issue with this technique is that if the ViewModel sends the event before the View is subscribed then it will never get it.

Here's an example using the round-trip style, similar to what @sockeqwe recommends. The trick here is to use a BehaviorSubject so the event will be there waiting for any subscriber.

class MyViewModel : ViewModel() {

    sealed class SnackbarState {
        data class Show(val message: String) : SnackbarState()
        object DontShow : SnackbarState()
    }

    private val showSnackbarPublisher = BehaviorSubject.create<SnackbarState>()
    val showSnackbarObservable: Observable<SnackbarState> = showSnackbarPublisher

    private fun doStuff() {
        showSnackbarPublisher.onNext(SnackbarState.Show("show this message"))
    }

    fun onSnackbarShown() {
        showSnackbarPublisher.onNext(SnackbarState.DontShow)
    }
}

class MyActivity : AppCompatActivity() {

    private fun subscribe() {
        vm.showSnackbarObservable
                .subscribe {
                    if (it is MyViewModel.SnackbarState.Show) {
                        Snackbar.make(rootView, it.message, Snackbar.LENGTH_LONG)
                                .show()
                        
                        vm.onSnackbarShown()
                    }
                }
    }
}
@Zhuinden

This comment has been minimized.

Show comment
Hide comment
@Zhuinden

Zhuinden Jul 21, 2018

One issue with this technique is that if the ViewModel sends the event before the View is subscribed then it will never get it. The trick here is to use a BehaviorSubject so the event will be there waiting for any subscriber.

The recommendation to use show/dontshow is overhead, just use a UnicastSubject instead of a PublishSubject

EDIT: Actually, UnicastSubject is more useless than I thought, and what we need is PublishRelay combined with the valve operator.

Zhuinden commented Jul 21, 2018

One issue with this technique is that if the ViewModel sends the event before the View is subscribed then it will never get it. The trick here is to use a BehaviorSubject so the event will be there waiting for any subscriber.

The recommendation to use show/dontshow is overhead, just use a UnicastSubject instead of a PublishSubject

EDIT: Actually, UnicastSubject is more useless than I thought, and what we need is PublishRelay combined with the valve operator.

@guelo

This comment has been minimized.

Show comment
Hide comment
@guelo

guelo Jul 21, 2018

UnicastSubject doesn't work because it doesn't allow re-subscriptions, which means that on rotation the new View won't be able to subscribe.

There is an operator UnicastWorkSubject in RxJava2Extensions that allows re-subscriptions. But it doesn't allow multiple observers at the same time, which can work, but there are situations where it won't work as evidenced by the people in this thread complaining that SingleLiveEvent doesn't allow multiple observers.

What I ended up doing in my project is using ReplaySubject along with the Event wrapper from @JoseAlcerreca's blog post. The ReplaySubject makes sure that no event is ever lost while the Event wrapper makes sure that no event is processed more than once. I used this approach because it's somewhat less boilerplate than the show/dontshow approach, even though I tend to agree with @sockeqwe that it is better to maintain single, immutable, unidirectional state.

guelo commented Jul 21, 2018

UnicastSubject doesn't work because it doesn't allow re-subscriptions, which means that on rotation the new View won't be able to subscribe.

There is an operator UnicastWorkSubject in RxJava2Extensions that allows re-subscriptions. But it doesn't allow multiple observers at the same time, which can work, but there are situations where it won't work as evidenced by the people in this thread complaining that SingleLiveEvent doesn't allow multiple observers.

What I ended up doing in my project is using ReplaySubject along with the Event wrapper from @JoseAlcerreca's blog post. The ReplaySubject makes sure that no event is ever lost while the Event wrapper makes sure that no event is processed more than once. I used this approach because it's somewhat less boilerplate than the show/dontshow approach, even though I tend to agree with @sockeqwe that it is better to maintain single, immutable, unidirectional state.

@Zhuinden

This comment has been minimized.

Show comment
Hide comment
@Zhuinden

Zhuinden Jul 21, 2018

@guelo wow, I overestimated the usefulness of UnicastSubject.

I guess what we really are looking for is what this answer is doing: https://stackoverflow.com/a/41272402/2413303

@guelo wow, I overestimated the usefulness of UnicastSubject.

I guess what we really are looking for is what this answer is doing: https://stackoverflow.com/a/41272402/2413303

@nicolasjafelle

This comment has been minimized.

Show comment
Hide comment
@nicolasjafelle

nicolasjafelle Jul 21, 2018

After all... is there any official/best solution to consume only one event? Is the Event class by @JoseAlcerreca plus the EventObserver by @JoseAlcerreca the best solution? For example:

In ViewModel:
var eventLiveData: MutableLiveData<Event<ErrorResponse>> = MutableLiveData()

In Activity or Fragment:

viewModel.eventLiveData.observe(this, EventObserver {
     it?.let {
          shortToast(it.message)
     }
})

nicolasjafelle commented Jul 21, 2018

After all... is there any official/best solution to consume only one event? Is the Event class by @JoseAlcerreca plus the EventObserver by @JoseAlcerreca the best solution? For example:

In ViewModel:
var eventLiveData: MutableLiveData<Event<ErrorResponse>> = MutableLiveData()

In Activity or Fragment:

viewModel.eventLiveData.observe(this, EventObserver {
     it?.let {
          shortToast(it.message)
     }
})
@guelo

This comment has been minimized.

Show comment
Hide comment
@guelo

guelo Jul 22, 2018

That seems to be the recommended solution but it has one big problem, if the ViewModel sends multiple events while the View is rotating then only the last event will be handled and the other events will be dropped. If instead you need the View to never miss an event and to handle them serially then I would recommend using rx's ReplaySubject instead of MutableLiveData along with Jose's Event wrapper.

In my case I had several types of View commands so I added a sealed class wrapper to end up with two wrappers.

class ViewModel {
  val viewCommandsPublisher = ReplaySubject.create<SingleReadEvent<ViewCommand>>()

  sealed class ViewCommand {
    data class Navigate1(val some: Data)
    data class FetchSomethingFromFramework(val some: OtherData)
    data class Snackbar(val message: String)
  }

  fun doStuff() {
    viewCommandsPublisher.onNext(SingleReadEvent(ViewCommand.Navigate1(data)))
  }
}

class View {
  fun observe() {
    viewModel.viewCommandsPublisher
      .subscribe { 
        val event = it.getContentIfNotHandled()
        if (event != null) {
          when (event) {
            is Navigate1 -> navigate(event)
            is FetchSomethingFromFramework -> fetch(event)
            is Snackbar -> snackbar(event)
         }}}
}

guelo commented Jul 22, 2018

That seems to be the recommended solution but it has one big problem, if the ViewModel sends multiple events while the View is rotating then only the last event will be handled and the other events will be dropped. If instead you need the View to never miss an event and to handle them serially then I would recommend using rx's ReplaySubject instead of MutableLiveData along with Jose's Event wrapper.

In my case I had several types of View commands so I added a sealed class wrapper to end up with two wrappers.

class ViewModel {
  val viewCommandsPublisher = ReplaySubject.create<SingleReadEvent<ViewCommand>>()

  sealed class ViewCommand {
    data class Navigate1(val some: Data)
    data class FetchSomethingFromFramework(val some: OtherData)
    data class Snackbar(val message: String)
  }

  fun doStuff() {
    viewCommandsPublisher.onNext(SingleReadEvent(ViewCommand.Navigate1(data)))
  }
}

class View {
  fun observe() {
    viewModel.viewCommandsPublisher
      .subscribe { 
        val event = it.getContentIfNotHandled()
        if (event != null) {
          when (event) {
            is Navigate1 -> navigate(event)
            is FetchSomethingFromFramework -> fetch(event)
            is Snackbar -> snackbar(event)
         }}}
}
@Zhuinden

This comment has been minimized.

Show comment
Hide comment
@Zhuinden

Zhuinden Jul 22, 2018

Okay, now just for the sake of completion, I'm adding that this is how I handled this scenario 3 years ago and it worked just fine without all these hacks:

public enum SingletonBus {
    INSTANCE;

    private static String TAG = SingletonBus.class.getSimpleName();

    private final Vector<Object> eventQueueBuffer = new Vector<>();

    private boolean paused;

    public <T> void post(final T event) {
            if (paused) {
                eventQueueBuffer.add(event);
            } else {
                EventBus.getDefault().post(event);
            }
    }

    public <T> void register(T subscriber) {
        EventBus.getDefault().register(subscriber);
    }

    public <T> void unregister(T subscriber) {
        EventBus.getDefault().unregister(subscriber);
    }

    public boolean isPaused() {
        return paused;
    }

    public void setPaused(boolean paused) {
        this.paused = paused;
        if (!paused) {
            Iterator<Object> eventIterator = eventQueueBuffer.iterator();
            while (eventIterator.hasNext()) {
                Object event = eventIterator.next();
                post(event);
                eventIterator.remove();
            }
        }
    }
}

public class BaseActivity extends AppCompatActivity {
     @Override
     public void onPostResume() {
          super.onPostResume();
          SingletonBus.INSTANCE.setPaused(false);
     }

     @Override
     public void onPause() {
          SingletonBus.INSTANCE.setPaused(true);
          super.onPause();
     }
}

Technically if ViewModel has its own pauseable bus, then problem is solved. 🙄

Zhuinden commented Jul 22, 2018

Okay, now just for the sake of completion, I'm adding that this is how I handled this scenario 3 years ago and it worked just fine without all these hacks:

public enum SingletonBus {
    INSTANCE;

    private static String TAG = SingletonBus.class.getSimpleName();

    private final Vector<Object> eventQueueBuffer = new Vector<>();

    private boolean paused;

    public <T> void post(final T event) {
            if (paused) {
                eventQueueBuffer.add(event);
            } else {
                EventBus.getDefault().post(event);
            }
    }

    public <T> void register(T subscriber) {
        EventBus.getDefault().register(subscriber);
    }

    public <T> void unregister(T subscriber) {
        EventBus.getDefault().unregister(subscriber);
    }

    public boolean isPaused() {
        return paused;
    }

    public void setPaused(boolean paused) {
        this.paused = paused;
        if (!paused) {
            Iterator<Object> eventIterator = eventQueueBuffer.iterator();
            while (eventIterator.hasNext()) {
                Object event = eventIterator.next();
                post(event);
                eventIterator.remove();
            }
        }
    }
}

public class BaseActivity extends AppCompatActivity {
     @Override
     public void onPostResume() {
          super.onPostResume();
          SingletonBus.INSTANCE.setPaused(false);
     }

     @Override
     public void onPause() {
          SingletonBus.INSTANCE.setPaused(true);
          super.onPause();
     }
}

Technically if ViewModel has its own pauseable bus, then problem is solved. 🙄

@nicolasjafelle

This comment has been minimized.

Show comment
Hide comment
@nicolasjafelle

nicolasjafelle Jul 23, 2018

@Zhuinden Isn't that an implementation of an event bus? like Otto from Square?

@Zhuinden Isn't that an implementation of an event bus? like Otto from Square?

@Zhuinden

This comment has been minimized.

Show comment
Hide comment
@Zhuinden

Zhuinden Jul 23, 2018

It uses greenrobot/EventBus at EventBus.getDefault().

It uses greenrobot/EventBus at EventBus.getDefault().

@gunhansancar

This comment has been minimized.

Show comment
Hide comment
@gunhansancar

gunhansancar Jul 23, 2018

Folks, please stop sharing unrelated solutions to this particular challenge. It is about LiveData and Android Architecture Components. Previously provided answers are already satisfying so maybe this issue should be closed?

Folks, please stop sharing unrelated solutions to this particular challenge. It is about LiveData and Android Architecture Components. Previously provided answers are already satisfying so maybe this issue should be closed?

@charlesng

This comment has been minimized.

Show comment
Hide comment
@charlesng

charlesng Aug 7, 2018

What If we have the static EventProvider and let it do something like EventBus post event. The structure is like the SingleLiveEvent but also use the EventBus Queue Structure.

`
public class EventProvider {

public static class EmitOnceEvent<T> extends MutableLiveData<T> {
    private ArrayList<Observer<T>> observerQueue = new ArrayList<>();
    private final AtomicBoolean mPending = new AtomicBoolean(false);
    @MainThread
    public void observe(LifecycleOwner owner, final Observer<T> observer) {
        observerQueue.add(observer);
        owner.getLifecycle().addObserver(new LifecycleObserver() {
            @OnLifecycleEvent(Lifecycle.Event.ON_DESTROY)
            public void onDestroy() {
                observerQueue.remove(observer);
            }
        });
        super.observe(owner, new Observer<T>() {
            @Override
            public void onChanged(@Nullable T t) {
                if (mPending.compareAndSet(true, false)) {
                    for (Observer<T> observer1 : observerQueue) {
                        observer1.onChanged(t);
                    }
                }
            }
        });
    }


    @MainThread
    public void setValue(@Nullable T t) {
        mPending.set(true);
        super.setValue(t);
    }

    /**
     * Used for cases where T is Void, to make calls cleaner.
     */
    @MainThread
    public void call() {
        setValue(null);
    }

}

private static EventProvider eventProvider;
private EmitOnceEvent<MyEvent> myEvent = new EmitOnceEvent<>();

private EventProvider() {

}

public static EventProvider getInstance() {
    if (eventProvider == null) {
        eventProvider = new EventProvider();
    }
    return eventProvider;
}

}

`

What If we have the static EventProvider and let it do something like EventBus post event. The structure is like the SingleLiveEvent but also use the EventBus Queue Structure.

`
public class EventProvider {

public static class EmitOnceEvent<T> extends MutableLiveData<T> {
    private ArrayList<Observer<T>> observerQueue = new ArrayList<>();
    private final AtomicBoolean mPending = new AtomicBoolean(false);
    @MainThread
    public void observe(LifecycleOwner owner, final Observer<T> observer) {
        observerQueue.add(observer);
        owner.getLifecycle().addObserver(new LifecycleObserver() {
            @OnLifecycleEvent(Lifecycle.Event.ON_DESTROY)
            public void onDestroy() {
                observerQueue.remove(observer);
            }
        });
        super.observe(owner, new Observer<T>() {
            @Override
            public void onChanged(@Nullable T t) {
                if (mPending.compareAndSet(true, false)) {
                    for (Observer<T> observer1 : observerQueue) {
                        observer1.onChanged(t);
                    }
                }
            }
        });
    }


    @MainThread
    public void setValue(@Nullable T t) {
        mPending.set(true);
        super.setValue(t);
    }

    /**
     * Used for cases where T is Void, to make calls cleaner.
     */
    @MainThread
    public void call() {
        setValue(null);
    }

}

private static EventProvider eventProvider;
private EmitOnceEvent<MyEvent> myEvent = new EmitOnceEvent<>();

private EventProvider() {

}

public static EventProvider getInstance() {
    if (eventProvider == null) {
        eventProvider = new EventProvider();
    }
    return eventProvider;
}

}

`

Sonphil added a commit to ApplETS/Notre-Dame-Android that referenced this issue Aug 15, 2018

Fix crash that could occur when user went back to MoreFragment and if…
… the About screen has been opened at least once

MoreFragment reacts to the his viewmodel LiveData and navigated to the AboutActivity. However, sometime (i.e. on configuration change) the fragment would subscribes to the LiveData again, get the value and triggers the navigation again. The event should only consume once! Here more information about this issue :
googlesamples/android-architecture-components#63
https://medium.com/google-developers/livedata-with-snackbar-navigation-and-other-events-the-singleliveevent-case-ac2622673150
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment