Androidエンジニアになって半年経ったので、コードレビューで指摘されたことをシュッとまとめる
Android Advent Calendar 2017 の15日目の記事です。
コードレビューしてますか?
Androidアプリのプログラマとして入社して半年が経ちました。入った頃よりはいくらか成長したかなと自負しているのですが、それはコードレビューで同僚や先輩から受けたコメントによって色々と学べたことが大きいのかなと思っています。
なので、このAdvent Calendarが良い節目だなと思ったので、まとめました。
Pull Request関連
差分はなるべく小さくすること
- 差分は、修正に意味がある範囲であれば、小さければ小さいほど良い。
- フォーマッタをかけた時の差分が大きいと修正の趣旨が分かり辛くなるので、事前に修正対象のファイルにフォーマットをかけただけのPull Requestを出す。
- (個人的なナイスコメント : diff全てに意味があるのがイケてるエンジニア)
コーディング全般
コードをなるべく減らせないか考えること
- 同じロジックの部分があれば、メソッドに切り出すことを検討する。
- 変数名で分かるものはコメントに書かなくて良い。後から修正するのが大変になる。
- コメントは何をしているかではなく、なぜそうしているかを書く。何をしているかのコメントは不要。
- (個人的なナイスコメント : 必要以上に未来予知せず、今の状態で必要なコードがあれば良いよ。)
なるべく頭を使わずに読めるコードを書くこと
- if条件式にはなるべく否定形を使わないようにしておくと頭を使わずに読めるコードになる。
- if条件式のネストが深いときは、早期リターンを検討する。
- 変数のスコープは可能な限り狭くする。
- 変数名のm、sプレフィックスは統一する。
- テストにセットするデータは実際に入りそうなテキストを入れる(ほげほげなど雑な文言を使わない)。そうした方がテストの意図が分かりやすい。
- (個人的なナイスコメント(口頭) : 人間の脳のキャッシュメモリってめっちゃ少ないから!)
Android/Java関連
なるべくテストを書けるようにすること
- Activityにロジックがあるときは、その処理をPresenterに切り出せないか考える。なるべくテストを書けるようにする。(今担当のアプリではMVPパターンを採用している)
- View毎の役割を考え、Viewの役割毎にCustomViewに切り分ける。CustomViewに切り分けた後、そのPresenterを書くことで、テストが書ける。
NullPointerExceptionに気をつけること
- 空文字チェックはTextUtils.isEmptyを使うと空文字の検出もできるし、null安全。
- メソッドの引数にnullが入らないことが明らかであるなら、@NonNullを付与する。
他
補足
他にも沢山コメントがあったのですが、typoなどのポカミスの指摘などは外し、これからも注意した方が良さそうな一般的なコメントをピックアップして見ました。
Androidに関する指摘が少ないのですが、コードというよりAndroidの知見に関することばかりだな、、、となってピックアップし出すとキリがない!! (これはただのAndroid Developersの目次だ!!)となったため省きました。
(その結果、Androidの技術に関することがテーマなのに、Androidに関する情報が少なくてすみません。m( )m)
良いお年を!!
