はじめに

「コードレビューが通りません...」

プログラマ・エンジニアになってから、しばらく付き合うことになる大きな壁だと思います。

この地獄からどうしたら抜け出せるのか？

スーパースキルの先輩エンジニアがレビュー時にボソッと言っていた観点を咀嚼し、数多のインプット・アウトプットを繰り返して、気づけばレビューをする側になっていました。

ここに気を付ければレビューで引っかかることが減った。自分がレビューするときはここに気を付けよう。というポイントを今回は紹介してみようと思います。

※コードに関してはそれぞれの思想があったり、必ずしもこれが正解といえるものの方が少ないと思います。考え方の１つとしてこういうのもあるよ程度に見てもらえれば幸いです。

コードレビューをするときの３つのポイント

細かいところをあげればきりがないですが、ここでは大きく分けて３つのポイントを紹介します。また、自分がコードを書き終わってプルリクエストを送る前に自分の書いたコードをこれらの観点で見返してみるのも良いと思います。

目的が達成されているか？

コードを書いたということは何かやりたいことがあったはず。コーディングする際の仕様があるのであれば、その仕様は網羅されているかみてみましょう。コードを記述した量が多くなるほど、意外と抜け落ちてしまうことは良くあります。

また、本当は実装する前に気づきたいですが「そもそもその仕様で目的は達成されるのか？」という観点でもコードは確認したいです。

最短工数で理解できるコードになっているか？

コードレビューがあるということは基本的にチームで開発しているはず。いいコードとは自分以外（数ヶ月後の自分かも）が最短工数で理解できるコードのことです。この辺はリーダブルコードに書いてあるので暗記できるくらい読み込みましょう。

また、チーム内のコードの記述に関しては共通認識を取れるように言語やフレームワークのコーディング規約に沿って記述しましょう。

ex)
php
cakephp

名前とロジックが一致しているか？

既存ロジックを修正する際によくなってしまうパターンです。変更によって想定する挙動や渡す引数などが変わってしまうと、その変更を知らないメンバーが流用した時など思わぬバグが発生したり、そのコードを理解するための余計なコストがかかったりします。追加時はもちろん、変更が加わったときには注意しましょう。

ex）　

- public function getRowById(int $id) {
-     return //idに紐づくデータを１件取得する。
- }
+ public function getRowById(array $ids) {
+     return //配列形式で渡したidに紐づくデータを複数取得する;
+ }

メソッド名と返される値が一致してない。
ex) getListByIds

番外編

３つと言ったのに４つ目ですね...

チーム内で決まっているコーディングルールは守りましょう。

ex) プライベートメソッドはパブリックメソッドと差別化する

- 
+ private function getFullName() {
+     return $this->firstName . $this->lastName;
+ }

プライベートメソッドは一目でわかるように下記のようにアンダーバーをつけてください
__getFullName()

ex) cssクラスの命名規則

.block {
}
.block__element1 {
}
-
+ .blockElement2 {
+ }

命名規則はBEMを採用しているのでクラス名は下記のようにしてください
block__Element2

さいごに

運動通信社は「日本を世界が憧れるスポーツ大国にする」というビジョンを達成するべく、一緒に働く仲間を募集しています！