この記事は 食べログアドベントカレンダー2022 の15日目の記事です🦌🎅
こんにちは。
食べログシステム本部 アプリ開発部 基盤チームの米山です。
私の所属する基盤チームは主に食べログアプリの保守や開発環境の改善を行なっています。
その中でも食べログAndroidアプリは、技術的負債を多く抱えているため、その返済を目的としたリアーキテクチャを数年前から実施しています。1
今年もいくつかの画面を対象にリアーキテクチャを実施しました。
中でも口コミを投稿/編集する画面(以下「口コミ投稿編集画面」という)のリアーキテクチャではリリース後にRatingBarに関連した様々な不具合を発生させてしまいました。
今回の記事では、これらの不具合の原因とそこから学んだことを紹介したいと思います。
今後RatingBar
を使う予定のある方はもちろんのこと、そうでない方にも有益な情報になれば幸いです。
背景
リアーキテクチャを実施する前の口コミ投稿編集画面は、以下のような状態で可読性が低くメンテナンスコストが高い状態でした。
- 口コミ投稿に関する大部分の処理を担っているManagerクラス(通称「神クラス」)と
Activity
やFragment
が密結合している。 Activity
やFragment
が肥大化している。- 内製のカスタムビュークラスのメンテナンスコストが高い。
そのため、今回のリアーキテクチャでは以下を実現することを主な目的としました。
- MVPパターンで作成し、Managerクラスと
Activity
やFragment
を直接やりとりさせないようにすること。 - Viewクラスの実装ではできるだけGoogle公式のレイアウトクラスを使用すること。
そして今回、口コミ投稿者が点数をつける際に使用する評価バーも内製のカスタムビュークラスから、Google公式のRatingBar
に置き換えることになりました。
このような方針が決まった後はチームメンバーで協力して「設計」→「実装」→「テスト」とタスクを消化し、2022年の夏頃にリアーキテクチャ版の口コミ投稿編集画面をリリースすることができました。
しかし、この後RatingBar
に関連した不具合がいくつか見つかることになります。
開発環境
今回のリアーキテクチャ実施時の開発環境は以下の通りです。
- targetSdkVersion 31
- Kotlin 1.5.31
不具合1:一部の端末でRatingBarのレイアウトが崩れる
内容
リアーキテクチャ版のリリースから数時間後、同僚のエンジニアからRatingBar
で実装した評価バーのレイアウトが崩れているという報告を受けました。
調査してみると以下の条件で発生することがわかりました。
条件
- OSのバージョン
- Android7.1.1あるいはAndroid8.0(Android6.0以下は食べログアプリではサポート外のため未調査)
- 端末設定のディスプレイ表示サイズ
- デフォルト設定以外のサイズ
リリース前にあらゆる端末、食べログアプリがサポートしている全OSバージョンでテストを行なっていましたが、端末設定のディスプレイ表示サイズが影響しているとは思いもしませんでした。
この不具合の原因と解決方法については次節で詳しく説明しますが、この不具合自体が大きな学びでした。
学んだこと
この不具合から学んだことは、個人の端末設定に目を向けることの重要性です。
今回の不具合は比較的稀なケースかもしれませんが、様々な方に使ってもらうことを考えると、ディスプレイの表示サイズの個人設定に目を向けることは見落としてはいけない視点だったように思えます。
しかし現状、レイアウトの確認は実装者がほぼ目視で行なっています。そのため、確認項目が多くなる分だけ確認に要するコストが増えることを意味します。開発者の負荷を上げずに、品質を向上させることは今後の課題と感じました。
不具合2:RatingBarの色が適切に設定されない
内容
不具合1のレイアウト崩れの件を調べていくと、progressBackgroundTintで星の背景色を設定するとレイアウト崩れが発生することがわかりました。
そのため、progressBackgroundTint
を使わずに、setTint
で以下のように星の背景色を設定するように修正しました。
val star = ratingbar.progressDrawable as LayerDrawable star.findDrawableByLayerId(android.R.id.background).setTint(/* tintColor */)
しかし、レイアウト崩れは解消したのですが、画面操作中に稀にRatingBar
の色が適切に設定されないという不具合が新たに発生してしまいました。
解決方法
数日間調査を続けたのですが、根本的な原因はわかりませんでした。
しかし、setTint
ではなくprogressBackgroundTint
で色を設定すれば、正常に色が設定されることは明らかでした。
そのため少々強引なやり方ですが、以下のようなXMLファイルを作成し、これをカスタムビュー化して使用することで不具合1、2を修正しました。
<!-- 星の背景用レーティングバーで操作不可 --> <RatingBar android:layout_width="wrap_content" android:layout_height="wrap_content" android:isIndicator="true" android:numStars="5" android:progressBackgroundTint="@android:color/transparent" android:progressTint="@color/gray" android:rating="5" /> <!-- 操作用のレーティングバー --> <RatingBar android:id="@+id/rating_bar" android:layout_width="wrap_content" android:layout_height="wrap_content" android:isIndicator="false" android:numStars="5" android:progressBackgroundTint="@android:color/transparent" android:rating="0" android:stepSize="0.5"/>
- 星の背景として表示するだけの
RatingBar
と、評価バーとして実際に操作できるRaitingBar
を重ねて配置する。 - 各々の
RatingBar
のprogressBackgroundTint
を透明に設定し、不具合1のように星の背景のレイアウトが崩れても見えないようにする。
その後の調査で分かったこと
上述の修正を行い修正版をリリースした後、何故このような事象が発生したのかについて継続して調べていました。
そして、RatingBar
の色が適切に設定されないという現象に関しては、以下の情報よりConstantState
が影響しているということがわかりました。
- StudyplusEngineeringBlog「RecyclerViewで Drawable に tint を設定する際は気をつけよう」
- Y.A.M の 雑記帳「Android Drawable Mutations」
- HiroYUKI Seto「DroidKaigi 2020 Lite - MDCの内部実装から学ぶ 表現力の高いViewの作り方」
ConstantState
は、Drawable
のアルファ値、colorStateList
、Tint
を保持し、ConstantState.newDrawable
で作成されたDrawable
間で共有されるようです。このConstantState.newDrawable
を使うDrawable
の作成方法はメモリ使用量を減らすため様々なwidgetクラス内部で使われているようで、アプリ実装者が気づかないうちに内部で個々のDrawable
のConstantState
が共有されることがあるようです。
そのため、今回の事象のように、全てもしくは一部のRatingBar
のDrawable
のConstantState
が共有されてしまい、適切に色が設定されなかったのだと思います。実際RatingBar
の親クラスであるProgressBarの実装でも、ConstantState.newDrawable
を使用している記述がありました。
このような事態を回避するためには、mutate を使用する必要があるようですが、既に多くのwidgetクラスにおいて、内部でmutate
を使用しているメソッドは用意してあるようです。
progressBackgroundTint
も内部でmutate
を呼んでいるメソッドであり、そのため正常に色が設定できたのだと思います。
しかし、なぜ不具合1のレイアウト崩れが発生したのか?
こちらの原因については、現在もよく分かっていません。Android8.1.0以降でこの問題が発生していないことから、特定のOSのバージョンでのみで発生する不具合であった可能性があります。
また、その後も調査したところmutate
を以下のように呼び出した上で、setTint
により色を設定すれば、上述の条件であってもレイアウト崩れが発生せず、色の設定も適切に行われることがわかりました。
val star = ratingbar.progressDrawable as LayerDrawable star.findDrawableByLayerId(android.R.id.background).mutate().setTint(/* tintColor */) star.findDrawableByLayerId(android.R.id.progress).mutate().setTint(/* tintColor */) star.findDrawableByLayerId(android.R.id.secondaryProgress).mutate().setTint(/* tintColor */)
ただし、この実装方法は十分に検証をしておらず別の不具合を生む可能性があるので、参考程度にしていただければと思います。
学んだこと
ConstantState
自体大きな学びではありましたが、以下に関しては特に大きな学びとなりました。
- 日頃から情報収集することが重要であること。
今回の事象が起きた際、ConstantState
を知っていさえすれば、問題解決をより早く行えたと思います。アプリ開発に必要な知識を全て深く理解することは難しいと思いますが、聞いたことがある程度の内容でも、何か起きた時の備えになるのではと感じました。
ちなみに、基盤チームでは輪読会という週1回1時間程度の勉強会を行なっています。内容は特に決めておらず、メンバーが興味があることや勉強したいことならなんでも良く、最近はDroidKaigi2022の動画を視聴しています。今回の一件で、改めてこのような場の重要性を感じましたし、個人でも無理のない範囲で情報収集していきたいと感じました。
不具合3:評価の点数が適切に反映されない
内容
リアーキテクチャ版の口コミ投稿編集画面のリリースから1週間ほど経った頃、「点数を変更しても、変更前の点数に戻される」という問い合わせをいただきました。
調査してみると、投稿後の口コミを後から編集する際に、Spinner
を使用して点数を4.7から4.5へ変更するような場合に発生することがわかりました。
具体的には以下のような操作を行うことで再現しました。
- 口コミ投稿編集画面の
Spinner
で点数を4.7にして口コミを投稿する。 - 再度口コミ投稿編集画面を開き、
Spinner
で点数を4.7から4.5へ変更して再投稿する。 - 再度口コミ投稿編集画面を開くと4.5に変更したはずの値が4.7に戻っている。
原因
Fragment
内のSpinner
のonItemSelected
内で、以下のような分岐処理を記述していました。この分岐処理は、点数に変化がない場合に、点数の更新などの後続の処理を行わないようにするため記述していました。しかし、今回この分岐処理が原因で点数の更新が行われなかったようでした。
override fun onItemSelected( parent: AdapterView<*>?, view: View?, position: Int, id: Long ) { ... // Spinnerで設定した0.1刻みの点数 val score = (floor(position - 1.0 + 10.0) * 0.1).toFloat() // Spinnerで設定した点数とRatingBarで保持している値を比較 if (ratingBar.rating != score) { // 0.5刻みの値に変えてRatingBarに値を設定する ratingBar.setRatingStarValue(context, score) // Presenter側の点数を更新する presenter.updateScore(score) } ...
なぜ更新処理が実行されなかったのか?
Spinner
では1.0から5.0まで0.1刻みで細かく点数を設定できるようになっていますが、RatingBar
では0.5刻みで手早く点数を設定できるようになっています。
そのため、Spinner
で0.5刻み以外の値に設定しても、RatingBar
では0.5刻みの値になるようにアプリ内部の処理で設定する必要があります。
実際、Spinner
で4.7に点数を設定した場合、RatingBar
では4.5を設定します。
そして、この場合以下のような状態になります。
- Presenter側で保持している点数の値は4.7
RatingBar
クラスが保持している値は4.5
そのため、評価を4.7に設定して口コミを投稿した後、再度口コミ投稿編集画面を開くと、Presenter側で保持している点数の値は4.7、RatingBar
クラスが保持している値は4.5といった状態で画面が表示されることになります。この場合、上のコードのような分岐処理があると、Spinner
で4.7から4.5へ値を変更したとしても分岐内の更新処理が呼ばれることはありません。
その結果、口コミを投稿しても点数の更新が行われず、変更前の点数のままとなったのです。
(同様に、3.7→3.5、1.3→1.0などといった変更でも更新処理が行われません)
学んだこと
この不具合に関してチーム内で議論した結果、以下のような意見が出て大きな学びになりました。
- 利用者がデータを更新するような画面では、更新後の値が適切に反映されているか入念に確認すること。
- 信頼できる唯一の情報源(SSOT)を意識すること。
1つ目に関しては、利用者にとって非常に重要な 点数をつける という機能のテストを入念に行わずにリリースしたという意味では、非常に大きな過ちだったと思います。今回のような複雑な画面や仕様の実装を行う場合、つい難しい実装箇所のテストばかりに目が行きがちです。しかし、利用者がデータを更新するような画面では、利用者目線に立ち、あらゆるユースケースを想定してテストケースを作成・実施することが重要です。そしてまた、このようなテストケースを作っておくことで、今後の改修の際にもリグレッションテストの項目として役立ち、障害の発生を防ぐことができるかもしれません。
2つ目のSSOTに関しては、Guide to app architecture にも記載されている内容です。このパターンのメリットとして、データの整合性の向上があげられています。SSOTを実現する方法は様々と思いますが、例えば今回の場合、評価の点数の値をViewModel
などに保持し、且つSSOTを意識していれば、上のコードのようにRatingBar
クラスの値を直接参照し別の情報源の値と比較するような処理を記述しなかったと思います。
また、関心の分離(Separation of concerns)も意識していれば、Fragment
にこのような処理を書くこともなく、テスタビリティも向上しユニットテストにより事前に不具合を見つけることができた可能性があります。
さらに、実装者だけでなくコードレビューにおけるレビューアも、これらの視点があれば事前に指摘できたかもしれません。
まとめ
RatingBar
の実装に関連した3つの不具合から、以下について学ぶことができました。
- 個人の端末設定に目を向けることが重要であること。
- 日頃から情報収集することが重要であること。
- 利用者がデータを更新するような画面では、更新後の値が適切に反映されているか入念に確認すること。
- 信頼できる唯一の情報源(SSOT)を意識すること。
今回紹介した内容が、何かお役に立てば幸いです。
明日は @aaknsk さんの「食べログのネット予約システムとは」です。お楽しみに!
- 詳しくは基盤チームリーダーのsadaさんの記事「食べログアプリでの技術的負債との向き合い方」をご覧ください↩