5 common mistakes when using Architecture Components
Subtle oversights with more or less serious consequences - even if you’re not making these mistakes it should be worth keeping them in mind to avoid running into some problems in future. This article covers:
- Leaking LiveData observers in Fragments
- Reloading data after every rotation
- Leaking ViewModels
- Exposing LiveData as mutable to Views
- Creating ViewModel’s dependencies after every configuration change
1. Leaking LiveData observers in Fragments
Fragments have tricky lifecycle and when a fragment gets detached and re-attached it is not always actually destroyed, for example, retained fragments are not destroyed during configuration changes. When this happens, the fragment’s instance survives and only its view gets destroyed, so onDestroy() is not called and DESTROYED state is not reached.
This means that if we start observing LiveData in onCreateView() or later (typically in onActivityCreated()) and pass Fragment as LifecycleOwner like:
we will wind up passing a new identical instance of Observer every time the fragment is re-attached, but LiveData won’t remove previous observers, because LifecycleOwner (Fragment) didn’t reach DESTROYED state. This eventually results in growing number of identical observers being active at the same time and same code from onChanged() being executed multiple times.
The issue was originally reported here and a more extensive explanation can be found here.
The recommended solution is to use fragment’s view lifecycle via getViewLifecycleOwner() or getViewLifecycleOwnerLiveData() which were added in Support Library 28.0.0 and AndroidX 1.0.0, so that LiveData will remove observers every time the fragment’s view is destroyed:
2. Reloading data after every rotation
We’re used to placing initialisation and setup logic in onCreate() in Activities (and by analogy onCreateView() or later in Fragments) so it might be tempting to trigger loading some data in ViewModels at this point as well. Depending on your logic, this might however cause reloading data after every rotation (even though a ViewModel was used), which in most cases is just senseless and unintended.
Examples:
The solution also depends on your logic. If for example, Repository would cache data then above code would be probably fine. Other solutions could also be:
- Using something similar to AbsentLiveData and starting loading only if data wasn’t set
- Starting loading data when it’s actually needed eg. in OnClickListener
- and probably the simplest: putting loading calls in ViewModel’s constructor and exposing pure getters
3. Leaking ViewModels
It was already clearly highlighted that we shouldn’t be passing View references to ViewModel
but we also should be cautious about passing references to ViewModels to other classes. After an Activity (or by analogy Fragment) is finished, ViewModel shouldn’t be referenced in any object that can outlive the Activity, so the ViewModel can be garbage collected.
The example leak can be passing in ViewModel a listener to Repository, which is Singleton scoped, and not clearing the listener afterwards:
Solution here could be removing the listener in onCleared() method, storing it as WeakReference in Repository, using LiveData to communicate between Repository and ViewModel - or basically anything that will suit you and ensure correct garbage collection.
4. Exposing LiveData as mutable to views
This one isn’t a bug, but it goes against separation of concerns.
Views - Fragments and Activities - shouldn’t be able of updating LiveData and thus their own state because that’s the responsibility of ViewModels. Views should be able to only observe LiveData.
Hence we should encapsulate access to MutableLiveData with eg. getters or backing properties:
5. Creating ViewModel’s dependencies after every configuration change
ViewModels survive configuration changes such as rotations, so creating their dependencies each time the change happens is simply redundant and can sometimes lead to unintended behaviour, especially if there’s logic put in the dependencies constructors.
While this might sound pretty obvious, it’s something that’s easy to overlook when using ViewModelFactory, which usually has the same dependencies as the ViewModel it creates.
ViewModelProvider preserves ViewModel instance, but not ViewModelFactory instance, so if we have code like this:
each time a configuration change happens, we will be creating a new instance of ViewModelFactory and therefore needlessly creating new instances of all its dependencies (assuming that they’re not somehow scoped).
Solution this time is to defer creation of the dependencies until create() method is actually invoked, because it is invoked only once during Activity/Fragment lifetime. We can achieve this by using, for example, lazy initialisation with eg. Providers:
