DroidKaigi / conference-app-2018

The Official Conference App for DroidKaigi 2018 Tokyo
Apache License 2.0
1.35k stars 333 forks source link

fix: StaffDataRepository.staff getter returns new instance each calls #674

Closed amay077 closed 6 years ago

amay077 commented 6 years ago

Overview (Required)

ブログを書いていたら、StaffDataRepository に2つの問題があるコードを見つけたので報告しておきます。 アプリの動作には問題となって表れない issue です。

1. loadStaff() を呼ばなくても staff の getter で検索処理が実行される

loadStaff() で検索処理が実行され、staff as Flowable を通じて結果が通じると期待しましたが、loadStaff() を呼ばずに staff プロパティを取得するだけで検索処理が実行されてしまいます。 なので、スタッフ画面の起動時に、スタッフ検索処理(getStaff())が2回実行されていました。

2. staff プロパティは get する度に新しい Flowable のインスタンスを生成してしまう

staff の getter や、 loadStaff() を呼ぶ度に Flowable インスタンスが再生成されています。 このため、 StaffViewModel 側の受信処理で

val staff: LiveData<Result<List<Staff>>> by lazy {
    repository.staff
            .toResult(schedulerProvider)
            .toLiveData()
}

と定義しても、その時の repository.staff は次回の loadStaff() 呼び出しで使われなくなってしまうので、結果として、loadStaff() を2度目以降の呼び出しには StaffViewModel.staff は無反応になります。アプリのスタッフ画面は loadStaff() が意図して複数回呼び出されるケースがないので、こちらも顕在化しません。

takahirom commented 6 years ago

👀

takahirom commented 6 years ago

Thanks! I didnt notice it🙏 LGTM