はじめに
「コードレビューが通りません...」
プログラマ・エンジニアになってから、しばらく付き合うことになる大きな壁だと思います。
この地獄からどうしたら抜け出せるのか？
スーパースキルの先輩エンジニアがレビュー時にボソッと言っていた観点を咀嚼し、数多のインプット・アウトプットを繰り返して、気づけばレビューをする側になっていました。
ここに気を付ければレビューで引っかかることが減った。自分がレビューするときはここに気を付けよう。というポイントを今回は紹介してみようと思います。
※コードに関してはそれぞれの思想があったり、必ずしもこれが正解といえるものの方が少ないと思います。考え方の１つとしてこういうのもあるよ程度に見てもらえれば幸いです。
コードレビューをするときの３つのポイント
細かいところをあげればきりがないですが、ここでは大きく分けて３つのポイントを紹介します。また、自分がコードを書き終わってプルリクエストを送る前に自分の書いたコードをこれらの観点で見返してみるのも良いと思います。
目的が達成されているか？
コードを書いたということは何かやりたいことがあったはず。コーディングする際の仕様があるのであれば、その仕様は網羅されているかみてみましょう。コードを記述した量が多くなるほど、意外と抜け落ちてしまうことは良くあります。
また、本当は実装する前に気づきたいですが「そもそもその仕様で目的は達成されるのか？」という観点でもコードは確認したいです。
最短工数で理解できるコードになっているか？
コードレビューがあるということは基本的にチームで開発しているはず。いいコードとは自分以外（数ヶ月後の自分かも）が最短工数で理解できるコードのことです。この辺はリーダブルコードに書いてあるので暗記できるくらい読み込みましょう。
また、チーム内のコードの記述に関しては共通認識を取れるように言語やフレームワークのコーディング規約に沿って記述しましょう。
名前とロジックが一致しているか？
既存ロジックを修正する際によくなってしまうパターンです。変更によって想定する挙動や渡す引数などが変わってしまうと、その変更を知らないメンバーが流用した時など思わぬバグが発生したり、そのコードを理解するための余計なコストがかかったりします。追加時はもちろん、変更が加わったときには注意しましょう。
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
さいごに
運動通信社は「日本を世界が憧れるスポーツ大国にする」というビジョンを達成するべく、一緒に働く仲間を募集しています！