Android アプリの開発で起きがちな問題を予防するためのコードレビューのチェックリストです。
チェックリストの方針
極力数少ない項目で多くの問題を防げること。
要は「最低限これを守ってくれるとまずまず OK だと思います」くらいのものです。
項目が少ないので重要な事柄に集中できますしレビュワの経験が少ない人もとっかかりやすいと思います。
全体的に、開発経験がそこまで豊富ではない方向けのチェックリストになっていると思います。
チェックリスト
全4項目です。
1. なるべく Activity にフィールドでフラグを持たせないこと
そうすべき理由
予想外のバグが発生するのを減らすためです。
下記の状況に陥ると問題が発生するのでこれらを回避します。
- 色々な箇所で色々なフラグを変更していてどこで何をしているかを把握できず、実装時にフラグの操作漏れが発生する
- 複数のフラグの組み合わせで挙動が変化するようになっていて、複雑なので、間違ったように理解してしまう
具体的には、1つの画面に複数の「モード」があるときに該当しやすいと思います。例えば写真撮影アプリに「連写モード」「タイマーモード」「通常モード」などがあり、それぞれの「モード」をフラグで表現している場合です。
「モード」は得てしてアプリのバージョンアップで追加されていくことが多いので、開発初期は問題がなくても、気がつくとフラグだらけでコードが理解できなくなってしまうことがあります
「通常モード」でしか「レトロ風フィルターモード」を使えなかったりというように、「モード」同士の関係性に制約があると、「モード」をフラグで管理するのはますます困難になります。
どのフラグをどのタイミングでどのような値にすべきかを把握しないとコードの変更が出来ないので、フラグが増えるほど問題が大きくなります。
とるべき改善策
- フラグではなくストラテジパターン等の別の手段で挙動を変える
- 1つの Activity に実装することを諦めて Activity を分割する
また、どうしてもフラグをなくせない場合は、次善策として下記のようにすると可読性を保ちやすいと思います。
- 各フラグに値を設定する箇所を Activity の中の1箇所に集める
- フラグ同士の排他関係などを Enum などを使って上手く整理する
2. Activity や AsyncTask からビューを細かく操作しすぎないこと
そのようにすべき理由
違反すると、コードを理解するのに長い時間がかかるようになってしまうからです。
具体的には下記の状況に陥って問題が発生しやすくなります。
- Activity や AsyncTask の中にコントローラ的なロジックやサーバとの通信やビューの操作などが入り乱れて、どこで何をしているのかが分からない。ビューの挙動を確認したいだけなのにあちこちのコードをさまようハメになる
- XML ではなく Java のどこかでビューが生成されている場合には、ビューの構造を理解したいだけなのに Java のコードをあちこちさまようハメになる
ユーザーの操作などに応じて画面の見た目が動的に激しく変化する場合にこの問題が生じがちです。
とるべき改善策
- 複数のまとまったビューを同じタイミングで操作しているなら、カスタムビューやフラグメントにまとめる
- カスタムビューの中のビューを直接 Activity からいじるのではなく、詳細な処理はカスタムビュー自身に任せる
- （つまり Activity からカスタムビューのメソッドを1つ叩けば、カスタムビュー自身がその中の各ビューを操作するようにする）
3. 性能的に問題がないこと
そのようにすべき理由
下記のようなアプリになると、ユーザがイライラしてしまいます。
- 処理が遅い
- メモリ不足で落ちる
- バッテリの消費が激しい
例えば下記のような場合にこの問題がおきます。
- ユーザがボタンを一つ押すと SQL が大量に投げられる。取得した大量のレコードをリスト表示する際に大量の画像を読み込む
- OpenGL で無闇やたらに描画を更新しようとしている（どうせ60FPSが限界なのに1msごとに更新をかけている、など）
とるべき改善策
- スペックが低い機種でテストして、問題の有無や対処の優先度を確認する
- 大量に SQL を投げているなら1つにまとめる
- Bitmap はサブサンプルしたものを読み込むようにしてメモリの消費を抑える
- そもそも無駄な処理をなくす
4. 画面が小さな端末、ピクセル密度が大きな端末や小さな端末でも、意図通りに表示される画面になっていること
そうすべき理由
表示が崩れるとかっこ悪いからです。
ありがちなのは下記のような問題です
- 画面が小さな端末でレイアウトが崩れている。ボタンが画面からはみ出していて操作が不可能になっている
- ピクセル密度が高い端末で、ボタンがものすごく小さく表示されている
- ボタンが小さくて押しづらい
とるべき改善策
- ビューのサイズやマージンなどは理由がない限りピクセルを使わない
- ボタンの当たり判定は見た目よりも大きくする（たとえば InsetDrawable を使う）
チェックリストの効果
レビュワとしても実装担当者としても、実感したのは、チェックリストの項目数はまずは少なくしておくほうが使いやすいということです。
少ないほうが最低限抑えるべき重要な事柄に集中しやすいからです。
チームやプロダクトの課題に合わせてチェックリストの内容は更新していくといいと思いました。