🙈

Android Architecture Componentsで犯しがちな5つの間違い【翻訳】

Keita Mori（ふぉれ）

2020/10/22に公開

Android

Kotlin

tech

この記事は著者 Michał Baran(@BaranMichal25) 氏の許可を得て翻訳したものです。

Original article: 5 common mistakes when using Architecture Components

大なり小なり重大な結果を引き起こす微妙な間違いは（たとえそのような間違いを犯してこなかったとしても）、将来起こるかも知れない問題を避けるために、頭に入れておいたほうが良い。

この記事では、以下の犯しがちな5つの間違いについて解説する。

FragmentでのLiveData observerのリーク
画面のローテーション後のデータリロード
ViewModelのリーク
MutableなLiveDataのViewへの公開
構成が変わる度に再作成されるViewModelの依存関係

1. FragmentでのLiveData observerのリーク

Fragmentはトリッキーなライフサイクルを持っていて、detachやre-attachで常に破棄される訳ではない。例えば、保持されたFragmentは構成の変更では破棄されない。これが起きると、Fragmentのインスタンスは生き残り、Viewだけが破棄される。そしてonDestory()は呼ばれず、DESTROYEDの状態に達しない。

これがどういうことかというと、例えば以下のようにLiveDataをonCreateView()やそれ以後（よくあるのはonActivityCreated()）でLifecycleOwnerとしてFragmentを渡して監視し始めたときに、問題が起きる。

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) })  // Risky: Passing Fragment as LifecycleOwner
    }

    ...
}

from gist

これはFragmentが再適用される度に新しいObserverインスタンスを渡すことになるが、LiveDataは前のObserverを破棄しない。なぜなら、LifecycleOwner（この例だとFragment）はDESTROYEDの状態になっていない。最終的にアクティブなObseverの数がどんどん増えていき、onChanged()で同じコードが何度も実行されることになる。

この問題はもともとここで報告され、さらに詳しい説明がここにある。

おすすめの解決方法はFragmentのView Lifecycleには、Support Library 28.0.0とAndroidX 1.0.0で追加された getViewLifecycleOwner() か getViewLifecycleOwnerLiveData() を使うことだ。これでLiveDataはFragmentのViewが破棄される度にObserverも破棄してくれる。

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) })    // Usually what we want: Passing Fragment's view as LifecycleOwner
    }

    ...
}

from gist

2. 画面のローテーション後のデータリロード

Activityの初期化やセットアップはonCreate()(FragmentだとonCreateView()かそれ以後)に書くことが多い。そしてそこに、ViewModelを介したデータのロードも書きたくなる。しかしそこに書いたことで、（ViewModelを使っていたとしても）画面がローテーションする度にデータのリロードが発生してしまう。これはほとんどの場合、無意識に意図せず起こってしまっている。

class ProductViewModel(
    private val repository: ProductRepository
) : ViewModel() {

    private val productDetails = MutableLiveData<Resource<ProductDetails>>()
    private val specialOffers = MutableLiveData<Resource<SpecialOffers>>()

    fun getProductsDetails(): LiveData<Resource<ProductDetails>> {
        repository.getProductDetails()  // Loading ProductDetails from network/database
        ...                             // Getting ProductDetails from repository and updating productDetails LiveData
        return productDetails
    }

    fun loadSpecialOffers() {
        repository.getSpecialOffers()   // Loading SpecialOffers from network/database
        ...                             // Getting SpecialOffers from repository and updating 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 { /*...*/ })  // (probable) Reloading product details after every rotation
        viewModel.loadSpecialOffers()                                       // (probable) Reloading special offers after every rotation
    }
}

from gist

この解決方法は実装による。例えば、Repositoryがデータをキャッシュしている場合、上のコードで問題ない。

AbsentLiveDataのようなものを使って、データがないときにだけロードする
実際に必要になったとき（OnClickListenerの中など）にロードする
最もシンプルなのが、ViewModelのコンストラクターでロード処理をして、純粋なGetterだけ公開する

class ProductViewModel(
    private val repository: ProductRepository
) : ViewModel() {

    private val productDetails = MutableLiveData<Resource<ProductDetails>>()
    private val specialOffers = MutableLiveData<Resource<SpecialOffers>>()

    init {
        loadProductsDetails()           // ViewModel is created only once during Activity/Fragment lifetime
    }

    private fun loadProductsDetails() { // private, just utility method to be invoked in constructor
        repository.getProductDetails()  // Loading ProductDetails from network/database
        ...                             // Getting ProductDetails from repository and updating productDetails LiveData
    }

    fun loadSpecialOffers() {           // public, intended to be invoked by other classes when needed
        repository.getSpecialOffers()   // Loading SpecialOffers from network/database
        ...                             // Getting SpecialOffers from repository and updating _specialOffers LiveData
    }

    fun getProductDetails(): LiveData<Resource<ProductDetails>> {   // Simple getter
        return productDetails
    }

    fun getSpecialOffers(): LiveData<Resource<SpecialOffers>> {     // Simple getter
        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 { /*...*/ })    // Just setting observer
        viewModel.getSpecialOffers().observe(this, Observer { /*...*/ })     // Just setting observer

        button_offers.setOnClickListener { viewModel.loadSpecialOffers() }
    }
}

from gist

3. ViewModelのリーク

ViewModelにViewの参照を渡すべきではないことは、すでに取り上げられている。

さらにViewModelに他のクラスの参照を渡すことにも、慎重になったほうが良い。Activity（やFragment）が終了したあと、ViewModelはガベージコレクタに破棄されるので、ViewModelはActivityよりも長生きするオブジェクトの参照を持つべきではない。

この例では、ViewModelでシングルトンのRepositoryにリスナーを渡していて、その後参照をクリアしていないので、リークする可能性がある。

@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<LocationRepository.Location>()
    private val repository = LocationRepository()

    init {
        repository.setOnLocationChangedListener {   // Risky: Passing listener (which holds reference to the MapViewModel)
            liveData.value = it                     // to singleton scoped LocationRepository
        }
    }
}

from gist

この解決方法では、Repositoryでは弱い参照で持ち、RepositoryとViewModel間でやりとりして、onCleared()メソッドでリスナーを破棄している。基本的には正しくガベージコレクションされればやりやすい方法で良い。

@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<LocationRepository.Location>()
    private val repository = LocationRepository()

    init {
        repository.setOnLocationChangedListener {   // Risky: Passing listener (which holds reference to the MapViewModel)
            liveData.value = it                     // to singleton scoped LocationRepository
        }
    }

    override onCleared() {                            // GOOD: Listener instance from above and MapViewModel
        repository.removeOnLocationChangedListener()  //       can now be garbage collected
    }
}

from gist

4. MutableなLiveDataのViewへの公開

これはバグではないが、関心事の分離に反している。FragmentやActivityなどのViewはLiveDataやこ自身の状態を更新できるべきではない。なぜならそれはViewModelの責務だからだ。ViewはLiveDataの監視だけをするべきだ。

したがって、MutableLiveDataは例えばGetterやバッキングプロパティなどでカプセル化すべきである。

class CatalogueViewModel : ViewModel() {

    // BAD: Exposing mutable LiveData
    val products = MutableLiveData<Products>()


    // GOOD: Encapsulate access to mutable LiveData through getter
    private val promotions = MutableLiveData<Promotions>()

    fun getPromotions(): LiveData<Promotions> = promotions


    // GOOD: Encapsulate access to mutable LiveData using backing property
    private val _offers = MutableLiveData<Offers>()
    val offers: LiveData<Offers> = _offers


    fun loadData(){
        products.value = loadProducts()     // Other classes can also set products value
        promotions.value = loadPromotions() // Only CatalogueViewModel can set promotions value
        _offers.value = loadOffers()        // Only CatalogueViewModel can set offers value
    }
}

from gist

5. 構成が変わる度に再作成されるViewModelの依存関係

ViewModelは画面のローテーションのような構成の変更があっても生き残る。そのため変更が生じる度に依存関係を作るのはシンプルに冗長だし、特に依存関係のコンストラクタのロジックで意図しない動きをしてしまうことがある。これはかなり明白かもしれないが、作成するViewModelと同じ依存関係を持つ、ViewModelFactoryを使うときに見落としやすい。

ViewModelProviderはViewModelのインスタンスを保持するが、ViewModelFactoryのインスタンスは保持しない。なので、このようなコードがあると問題が起きる。


class MoviesViewModel(
    private val repository: MoviesRepository,
    private val stringProvider: StringProvider,
    private val authorisationService: AuthorisationService
) : ViewModel() {

    ...
}


class MoviesViewModelFactory(   // We need to create instances of below dependencies to create instance of MoviesViewModelFactory
    private val repository: MoviesRepository,
    private val stringProvider: StringProvider,
    private val authorisationService: AuthorisationService
) : ViewModelProvider.Factory {

    override fun <T : ViewModel> create(modelClass: Class<T>): T {  // but this method is called by ViewModelProvider only if ViewModel wasn't already created
        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?) {    // Called each time Activity is recreated
        super.onCreate(savedInstanceState)
        setContentView(R.layout.activity_movies)

        injectDependencies() // Creating new instance of MoviesViewModelFactory

        viewModel = ViewModelProviders.of(this, viewModelFactory).get(MoviesViewModel::class.java)
    }

    ...
}

from gist

構成の変更がある度に、ViewModelFactoryの新しいインスタンスが作成され、それに従って不必要なすべての依存関係のインスタンスが作成される（それが何らかの形でスコープされてないと仮定）。

このときの解決策はcreate()メソッドが実際に呼び出されるまで、依存関係の作成をしないようにすることだ。そうすればActivityとFragmentが生きている間に1回しか呼び出されない。例えばProviderなどを使って初期化を遅延させることで解決できる。

class MoviesViewModel(
    private val repository: MoviesRepository,
    private val stringProvider: StringProvider,
    private val authorisationService: AuthorisationService
) : ViewModel() {

    ...
}


class MoviesViewModelFactory(
    private val repository: Provider<MoviesRepository>,             // Passing Providers here
    private val stringProvider: Provider<StringProvider>,           // instead of passing directly dependencies
    private val authorisationService: Provider<AuthorisationService>
) : ViewModelProvider.Factory {

    override fun <T : ViewModel> create(modelClass: Class<T>): T {  // This method is called by ViewModelProvider only if ViewModel wasn't already created
        return MoviesViewModel(repository.get(),
                               stringProvider.get(),                // Deferred creating dependencies only if new insance of ViewModel is needed
                               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() // Creating new instance of MoviesViewModelFactory

        viewModel = ViewModelProviders.of(this, viewModelFactory).get(MoviesViewModel::class.java)
    }

    ...
}

from gist

参考資料

この記事は著者 Michał Baran(@BaranMichal25) 氏の許可を得て翻訳したものです。間違いがある場合はコメントか、@d_forestまでお願いします。

Original article: 5 common mistakes when using Architecture Components

Thank you Michał!

GitHubで編集を提案

PYPublication

PY is a Tokyo-based tech studio that creates everything fun and fair.

1. FragmentでのLiveData observerのリーク

2. 画面のローテーション後のデータリロード

3. ViewModelのリーク

4. MutableなLiveDataのViewへの公開

5. 構成が変わる度に再作成されるViewModelの依存関係

参考資料

Discussion