5 common mistakes when using Android architectural components

Even if you do not make these mistakes, it is worth remembering them so as not to encounter some problems in the future.

1. Leakage of LiveData observers in fragments

Fragments have a complex life cycle, and when a fragment disconnects and rejoins an Activity, it is not always destroyed. For example, saved fragments are not destroyed during configuration changes. When this happens, the fragment instance remains, and only its View is destroyed, so onDestroy()it is not called and the DESTROYED state is not reached.

This means that if we start watching LiveData at onCreateView()or later (usually at onActivityCreated()) and pass the fragment as a LifecycleOwner:

class BooksFragment: Fragment() {
    private lateinit var viewModel: BooksViewModel
    override fun onCreateView(inflater: LayoutInflater, container: ViewGroup?, savedInstanceState: Bundle?): View? {
        return inflater.inflate(R.layout.fragment_books, container)
    }
    override fun onActivityCreated(savedInstanceState: Bundle?) {
        super.onActivityCreated(savedInstanceState)
        viewModel = ViewModelProviders.of(this).get(BooksViewModel::class.java)
        viewModel.liveData.observe(this, Observer { updateViews(it) })  // передача фрагмента как LifecycleOwner
    }
    ...
}

then we will transmit a new identical instance of Observer every time a fragment re-joins, but LiveData will not delete previous observers because LifecycleOwner (fragment) has not reached the DESTROYED state. This ultimately leads to the fact that the number of identical and simultaneously active observers is growing, and the same code from onChanged()is executed several times.

The issue was originally reported here , and a more detailed explanation can be found here .

The recommended solution is to use getViewLifecycleOwner () or getViewLifecycleOwnerLiveData () from the fragment life cycle, which were added to the support library 28.0.0 and AndroidX 1.0.0, so that LiveData will delete observers every time the View fragment is destroyed:

class BooksFragment : Fragment() {
    ...
    override fun onActivityCreated(savedInstanceState: Bundle?) {
        super.onActivityCreated(savedInstanceState)
        viewModel = ViewModelProviders.of(this).get(BooksViewModel::class.java)
        viewModel.liveData.observe(viewLifecycleOwner, Observer { updateViews(it) })
    }
    ...
}

2. Rebooting data after each screen rotation

We are used to putting the initialization and configuration logic in onCreate()in Activity (and by analogy in onCreateView()or later in fragments), so at this stage it may be tempting to initiate the loading of some data in ViewModels. However, depending on your logic, this can lead to a data reload after each rotation of the screen (even if the ViewModel was used), which in most cases is simply pointless.

Example:

class ProductViewModel(
    private val repository: ProductRepository
) : ViewModel() {
    private val productDetails = MutableLiveData>()
    private val specialOffers = MutableLiveData>()
    fun getProductsDetails(): LiveData> {
        repository.getProductDetails()  // Загрузка ProductDetails
        ...                             // Получение ProductDetails и обновление productDetails LiveData
        return productDetails
    }
    fun loadSpecialOffers() {
        repository.getSpecialOffers()   // Загрузка SpecialOffers
        ...                             // Получение SpecialOffers и обновление specialOffers LiveData
    }
}
class ProductActivity : AppCompatActivity() {
    lateinit var productViewModelFactory: ProductViewModelFactory
    override fun onCreate(savedInstanceState: Bundle?) {
        super.onCreate(savedInstanceState)
        val viewModel = ViewModelProviders.of(this, productViewModelFactory).get(ProductViewModel::class.java)
        viewModel.getProductsDetails().observe(this, Observer { /*...*/ })  // (возможно) Повторный запрос productDetails после поворота экрана
        viewModel.loadSpecialOffers()                                       // (возможно) Повторный запрос specialOffers после поворота экрана
    }
}

The decision also depends on your logic. If the repository will cache data, then the above code will probably work correctly. Also, this problem can be solved in other ways:

• Use something similar to AbsentLiveData and start downloading only if data has not been received;
• Start downloading data when it’s really necessary. For example, in OnClickListener;
• And probably the simplest: put load calls in the ViewModel constructor and use simple getters:

class ProductViewModel(
    private val repository: ProductRepository
) : ViewModel() {
    private val productDetails = MutableLiveData>()
    private val specialOffers = MutableLiveData>()
    init {
        loadProductsDetails()           // ViewModel создаётся только один раз в ходе жизненных циклов Activity или фрагмента
    }
    private fun loadProductsDetails() {
        repository.getProductDetails()  
        ...                             
    }
    fun loadSpecialOffers() {           
        repository.getSpecialOffers()   
        ...                            
    }
    fun getProductDetails(): LiveData> { 
        return productDetails
    }
    fun getSpecialOffers(): LiveData> {    
        return specialOffers
    }
}
class ProductActivity : AppCompatActivity() {
    lateinit var productViewModelFactory: ProductViewModelFactory
    override fun onCreate(savedInstanceState: Bundle?) {
        super.onCreate(savedInstanceState)
        val viewModel = ViewModelProviders.of(this, productViewModelFactory).get(ProductViewModel::class.java)
        viewModel.getProductDetails().observe(this, Observer { /*...*/ })    
        viewModel.getSpecialOffers().observe(this, Observer { /*...*/ })   
        button_offers.setOnClickListener { viewModel.loadSpecialOffers() }
    }
}

3. Leak ViewModels

Architecture developers made it clear that we should not pass View links to the ViewModel.

but we must also be careful when passing references to ViewModels to other classes. After the Activity (or fragment) is completed, the ViewModel cannot be referenced to any object that the Activity can survive, so that the ViewModel can be destroyed by the garbage collector.

In this leak example, the Repository listener, written in Singleton style, is passed to the ViewModel. Subsequently, the listener is not destroyed:

@Singleton
class LocationRepository() {
    private var listener: ((Location) -> Unit)? = null
    fun setOnLocationChangedListener(listener: (Location) -> Unit) {
        this.listener = listener
    }
    private fun onLocationUpdated(location: Location) {
        listener?.invoke(location)
    }
}
class MapViewModel: AutoClearViewModel() {
    private val liveData = MutableLiveData()
    private val repository = LocationRepository()
    init {
        repository.setOnLocationChangedListener {   
            liveData.value = it              
        }
    }
}

The solution here could be to remove the listener in the method onCleared(), save it as WeakReferencein a repository, or use LiveData to communicate between the repository and the ViewModel:

@Singleton
class LocationRepository() {
    private var listener: ((Location) -> Unit)? = null
    fun setOnLocationChangedListener(listener: (Location) -> Unit) {
        this.listener = listener
    }
    fun removeOnLocationChangedListener() {
        this.listener = null
    }
    private fun onLocationUpdated(location: Location) {
        listener?.invoke(location)
    }
}
class MapViewModel: AutoClearViewModel() {
    private val liveData = MutableLiveData()
    private val repository = LocationRepository()
    init {
        repository.setOnLocationChangedListener {   
            liveData.value = it                    
        }
    }
    override onCleared() {                           
        repository.removeOnLocationChangedListener() 
    }  
}

4. Allow View to modify LiveData

This is not a bug, but it contradicts the separation of interests. View - fragments and Activity - should not be able to update LiveData and, therefore, its own state, because it is the responsibility of ViewModels. View should be able to only watch LiveData.

Therefore, we must encapsulate access to MutableLiveData:

class CatalogueViewModel : ViewModel() {
    // ПЛОХО: позволять изменять MutableLiveData
    val products = MutableLiveData()
    // ХОРОШО: инкапсулировать доступ к MutableLiveData
    private val promotions = MutableLiveData()
    fun getPromotions(): LiveData = promotions
    // ХОРОШО: инкапсулировать доступ к MutableLiveData
    private val _offers = MutableLiveData()
    val offers: LiveData = _offers
    fun loadData(){
        products.value = loadProducts()     // Другие классы смогут изменять products
        promotions.value = loadPromotions() // Только CatalogueViewModel может изменять promotions
        _offers.value = loadOffers()        // Only CatalogueViewModel может изменять _offers
    }
}

5. Creating ViewModel dependencies after each configuration change

ViewModels withstand configuration changes, such as screen rotation, so if you create them every time a change occurs, it can sometimes lead to unpredictable behavior, especially if some logic is built into the designer.

Although this may seem fairly obvious, it’s something that is easy to overlook when using the ViewModelFactory, which usually has the same dependencies as the ViewModel it creates.

ViewModelProvider saves an instance of ViewModel, but not an instance of ViewModelFactory, so if we have this code:

class MoviesViewModel(
    private val repository: MoviesRepository,
    private val stringProvider: StringProvider,
    private val authorisationService: AuthorisationService
) : ViewModel() {
    ...
}
class MoviesViewModelFactory( 
    private val repository: MoviesRepository,
    private val stringProvider: StringProvider,
    private val authorisationService: AuthorisationService
) : ViewModelProvider.Factory {
    override fun  create(modelClass: Class): T { 
        return MoviesViewModel(repository, stringProvider, authorisationService) as T
    }
}
class MoviesActivity : AppCompatActivity() {
    @Inject
    lateinit var viewModelFactory: MoviesViewModelFactory
    private lateinit var viewModel: MoviesViewModel
    override fun onCreate(savedInstanceState: Bundle?) {  
        super.onCreate(savedInstanceState)
        setContentView(R.layout.activity_movies)
        injectDependencies()
        viewModel = ViewModelProviders.of(this, viewModelFactory).get(MoviesViewModel::class.java)
    }
    ...
}

then every time a configuration change occurs, we will create a new instance of ViewModelFactory and, therefore, without special need to create new instances of all its dependencies (provided that they are not limited in any way).

class MoviesViewModel(
    private val repository: MoviesRepository,
    private val stringProvider: StringProvider,
    private val authorisationService: AuthorisationService
) : ViewModel() {
    ...
}
class MoviesViewModelFactory(
    private val repository: Provider,             
    private val stringProvider: Provider,         
    private val authorisationService: Provider
) : ViewModelProvider.Factory {
    override fun  create(modelClass: Class): T {  
        return MoviesViewModel(repository.get(),                    
                               stringProvider.get(),                
                               authorisationService.get()
                              ) as T
    }
}
class MoviesActivity : AppCompatActivity() {
    @Inject
    lateinit var viewModelFactory: MoviesViewModelFactory
    private lateinit var viewModel: MoviesViewModel
    override fun onCreate(savedInstanceState: Bundle?) {
        super.onCreate(savedInstanceState)
        setContentView(R.layout.activity_movies)
        injectDependencies()
        viewModel = ViewModelProviders.of(this, viewModelFactory).get(MoviesViewModel::class.java)
    }
    ...
}