3

The suggested way to implement ViewModel is to expose the changing data by using LiveData objects to activities, fragments and views. There are cases, when LiveData is not an ideal answer or no answer at all.

The natural alternative would be, to apply the observer pattern to the ViewModel, make it an observable. When registering observers to the ViewModel, the ViewModel will hold callback references to notify the observers.

The documentation says, a ViewModel must not hold references to activities, fragments or views. The only answer to the question "why" I found is, that this may cause memory leaks. Then how about cleaning up the references to avoid memory leaks?

For views this is a difficulty. There is no defined moment, when the view goes away. But activities and fragments have a defined lifecycle. So there are places to unregister as observers.

What do you think? Is it valid to register activities as observers to ViewModels if you take care to always unregister them? Did you hit upon any valid information about this question?

I set a small reward for the best answer. It's not because I think it a recommended solution (as it does not work with views). I just want to know and extend my options.

public class ExampleViewModel extends ViewModel {

    public interface OnEndListener {
        public void onEnd();
    }

    private List<OnEndListener> onEndListeners = new ArrayList<>();

    public void setOnEndListener(OnEndListener onEndListener) {
        onEndListeners.add(onEndListener);
    }

    public void removeOnEndListener(OnEndListener onEndListener) {
        onEndListeners.remove(onEndListener);
    }

    public void somethingHappens() {
        for (OnEndListener onEndListener: new ArrayList<OnEndListener>(onEndListeners) ) {
            onEndListener.onEnd();
        }
    }
}

public class ExampleActivity extends AppCompatActivity {

    ExampleViewModel exampleViewModel;
    ExampleViewModel.OnEndListener onEndListener;

    @Override
    protected void onCreate(@Nullable Bundle savedInstanceState) {
        super.onCreate(savedInstanceState);
        onEndListener = new ExampleViewModel.OnEndListener() {
            @Override
            public void onEnd() {
                finish();
            }
        };
        exampleViewModel = ViewModelProviders.of(this).get(ExampleViewModel.class);
        exampleViewModel.setOnEndListener(onEndListener);

    }

    @Override
    protected void onDestroy() {
        super.onDestroy();
        exampleViewModel.removeOnEndListener(onEndListener);
    }
}
Blcknx
  • 1,921
  • 1
  • 12
  • 38
  • *There are cases, when LiveData is not an ideal answer or no answer at all.* But what kind of usecase are you trying to solve? – Enzokie Apr 24 '18 at 11:56
  • @Enzokie Here are two example cases: https://stackoverflow.com/questions/49077746/how-to-finish-activity-from-viewmodel-using-arndroid-archtecture-components https://stackoverflow.com/questions/49894570/android-how-to-generate-composed-view-models – Blcknx Apr 24 '18 at 12:02
  • 1
    The problem you want to fix with this solution is not clear to me. I think that `must not hold references to activities` does not mean that you can't use observers. It is actually what you do when using LiveData: you register observers. `must not hold references to activities` means that you should not use the ViewModel like a Presenter that holds a ref to the view that can't be unsubscribed. The reason is not the leaks but the fact that when the view dies you have to kill the presenter as well. Of course, you can do tricky things like in the Mosby lib but that is not exactly MVP in my opinion – kingston May 06 '18 at 18:39

1 Answers1

2

To ask "am I allowed..." is not really a useful question, IMO. The docs are clear that what you are suggesting is discouraged and why. That said, I expect that your code would probably work as expected and is therefore "allowed" (i.e. not prevented by a technical constraint).

One possible gotcha scenario: InstanceA of ExampleActivity is started and kicks off some long-running task on the ExampleViewModel. Then, before the task completes, the device is rotated and InstanceA is destroyed because of the configuration change. Then, in between the time when InstanceA is destroyed and a new InstanceB is created, the long-running task completes and your view model calls onEndListener.onEnd(). Except: Oh no! The onEndListener is null because it was cleared when InstanceA was destroyed and hasn't yet been set by InstanceB: NullPointerException

ViewModel was designed (in part) precisely to handle edge cases like the gotcha scenario above. So instead of working against the intended use of the ViewModel, why not just use the tools it offers along with LiveData to accomplish the same thing? (And with less code, I might add.)

public class ExampleActivity extends AppCompatActivity {

    ExampleViewModel exampleViewModel;

    @Override
    protected void onCreate(@Nullable Bundle savedInstanceState) {
        super.onCreate(savedInstanceState);
        exampleViewModel = ViewModelProviders.of(this).get(ExampleViewModel.class);
        exampleViewModel.getOnEndLive().observe(this, new Observer<Boolean>() {
            @Override
            public void onChanged(@Nullable Boolean onEnd) {
                if (onEnd != null && onEnd) {
                    finish();
                }
            }
        });

    }
}

public class ExampleViewModel extends ViewModel {

    private MutableLiveData<Boolean> onEndLive = new MutableLiveData<>();

    public MutableLiveData<Boolean> getOnEndLive() {
        return onEndLive;
    }

    public void somethingHappens() {
        onEndLive.setValue(true);
    }
}

Think of the LiveData in this case not as actual "data" per se, but as a signal that you can pass from your ViewModel to your Activity. I use this pattern all the time.

mikejonesguy
  • 9,779
  • 2
  • 35
  • 49
  • You gotcha is an interesting point. Does `LiveData` guaranty, that none such a gap is happening between the first and the second activity during rotation? That at least one activities `finish()` method is always reachable? Is this guarantee given by overlapping? If yes, is this overlapping not also valid for my example code? I updated my code to allow for multiple observers to allow for overlapping. – Blcknx May 03 '18 at 08:15
  • I honestly don't know if your updated code would solve the problem or not. I don't know how the timing of the `Activity` lifecycle works, but it seems reasonable to assume that if you have a task on a background thread, it could theoretically complete in between `InstanceA.onDestroy()` and `InstanceB.onCreate()`. – mikejonesguy May 03 '18 at 14:47
  • I *believe* that the `LiveData`/`ViewModel` combo handles this scenario correctly (i.e., the observer on InstanceB is triggered immediately when it comes online), but I haven't tested it. Worst case, you could add something in onCreate() that checks the value of `onEndLive` and reacts appropriately if it's already set, but I *think* that's not necessary. – mikejonesguy May 03 '18 at 14:58
  • I should clarify one thing in case you're not aware: the `ViewModel` and its property `onEndLive`) *survive* the `Activity`'s configuration change. So there is no InstanceB of the `ViewModel` -- it's the same, singular instance. That's part of the power of the `ViewModel`, and that is why your question about overlapping is not relevant in my example code. My assumption is that the activities do not overlap, but because the `ViewModel` persists through the configuration change, it doesn't matter. – mikejonesguy May 03 '18 at 15:08
  • Yes it is relevant. The single `ViewModel` instance survives. There is a handover of the activities. For all solutions there is a danger of a gap (no call to `finish()`) or notification of both activities. It depends on the details of the handover. The `ViewModel` `LiveData` solution is more complex as both objects are related to the lifecycle and both need to do the handover upon rotation. If they did it well, the handover should be cleanly synchronised, so that `LiveData` will notify observers of exactly one activity at each point of time of the rotation. – Blcknx May 03 '18 at 16:01
  • We'll have to agree to disagree. IMO, ViewModel/LiveData solution is simple, elegant, and better in this situation. I think I've made a pretty good case for that opinion, but ultimately it's not my job to convince you of that. Best wishes to you and happy coding. – mikejonesguy May 03 '18 at 16:11
  • It's not about mission. It's about understanding. – Blcknx May 03 '18 at 21:00