みなさんのチームでは、コードレビューは行っているでしょうか？
メンバーズエッジカンパニーではリモートチームによるアジャイル開発を行っており、自分のチームでは言語にPython、フレームワークにDjangoを使った、業務系システムを作っています。
自分のチームでもコードレビューを行っていますが、自分はチームの立ち上げから関わり、かつプログラマーとしての経験も長いため、レビューをする立場が多いです。
この記事では、チーム開発においてのコードレビューの意義と手段について紹介していきます。
株式会社メンバーズ メンバーズエッジカンパニー Webエンジニア
2019年中途入社。
神戸オフィスにて1年間の修行後、愛媛県にてフル在宅勤務中。
好きなハードはNeXTcubeです。
コードレビューの意義
コードレビューをする意義は何でしょうか。自分は4つあると考えています。
1つ目は、バグの発見です。これは言うまでもないでしょう。
2つ目は、保守性、特に変更容易性の確保です。システムは作って終わりではなく、次々と変更が加わります。そのため、変更しやすいコードの維持が必要です。
3つ目は、属人化を避けるためです。1人しか理解していないコードがあると、その人にしか変更できなくなります。その人に業務が集中してしまいます。その人が不在のときに困ります。
4つ目は、チーム内コミュニケーションのためです。コードレビューはコードを通した対話です。一方的に問題点を指摘するのではなく、レビューする側も学び、ときには疑問点をぶつけてよりよい解決案を導き出します。
では、このような意義に沿って効果的にレビューを行うために、どのような環境が必要でしょうか？
CI環境とエラー検知を整える
コードレビューはものすごく時間と神経を使う作業です。なぜなら、他の人が書いたコードの意図を読み取りつつも、批判的に考えないといけないからです。
そのため重要なことに集中できるように、些細な問題、例えば演算子の前のスペース不足、未使用の変数などはツールで自動的にチェックさせるのがいいでしょう。また、開発者には自分でテストコードを書いてもらっています。
これらの目的のために、自分のチームでは次のツールを導入しています。
- black(自動整形ツール)
- flake8(静的解析ツール)
- pytest(テストフレームワーク)
そしてこの3つをCircleCIで自動実行して、エラーがないことを開発者に確認してもらっています。すなわち、継続的インテグレーション(CI)環境を整えています。
また、コードレビューに使える時間は限られているため、プログラムに問題があってエラーが起きたときに、すぐに検知できるシステムが必要です。自分のチームでは、エラー検知にSentryを使用しています。
コードレビューの意義に沿ったレビュー手法
1. バグの発見
システム開発においては、後工程になるほどバグの修正コストが高くなります。そのため、コードレビューでバグを全て取り除きたいです。その一方で、やりたいことに対して、時間は常に足りないというジレンマがあります。
そのため、自分は重要度に応じてレビュー精度を変えています。特に次の3つは問題が起きたときの被害が大きいため、集中して見ています。
- セキュリティ上の問題がないか
- モデル(テーブル)の設計に問題がないか
- バッチ処理ではリカバリーを考慮しているか
自分はコードを見るときはまず「素直な書き方」かどうかを見ます。業務系システムの場合、凝ったアルゴリズムはほとんど使いません。そのため「変な書き方」を見つけたら、コードを書いた人に質問します。そしてバグではなく意図的に変な書き方をしていた場合は、その理由をコメントとして残してもらいます。
また、エラー処理の実装も確認します。なぜなら、エラー処理は開発者の意識から外れやすく、問題があっても気づきにくいからです。
さらに「一緒に直すべきところが他にないか」「この仕様でユーザが本当に満足するか」といった、一つ上の視点で考えています。
2. 変更容易性の確保
バグの発見と同じく重要なのが、コードの変更容易性の確保です。なぜなら、機能追加にしろバグ修正にしろ、変更しやすいコードと、変更しづらいコードでは生産性に大きな差が出るからです。
そのためにまず、開発者に自分でテストコードを書いてもらいます。テストコードがあることで、安心してリファクタリングや機能追加ができるからです。「テストがないコードはレガシーコード」という言葉もあるように、現代の開発ではテストコードは必須です。
それから先程の「バグの発見」で書いたように、「素直な書き方」かどうかを確認します。最短で理解できるコードが、もっとも変更しやすいコードだからです。
3. コードレビューを属人化しない
効率だけで言えば、経験の長い人、あるいはその機能に1番詳しい人がコードレビューを行うのが最適でしょう。しかしそれではコードレビュー業務が特定の人に偏ってしまいます。
一方で、プログラマーとしての経験が短い人、チームに入って間もない人がコードレビューを行うのも難しいでしょう。
そのため、いろいろな工夫をしています。
- コーディング規約を書く
- 2人で機能を開発して、相互にコードレビューする
- 難易度によってレビュアー、レビュー人数を変える
まずは、コードレビューの基準となる、コーディング規約を書きます。ただし全ての規約を書くと大変なので、プロジェクト固有のルールや、何度もコードレビューで指摘されるもののみでも十分です。
次に、大きめの機能開発は2人ペアで作業します。ベテランと若手のペアがいいでしょう。そして細かくブランチを切って作業を行い、相互にコードレビューします。最後に別のベテランがコードレビューすることで、若手に経験を積ませつつ、全体として品質を確保します。
そして最後に、難易度に応じてコードレビューを行うメンバーや人数を変えます。難易度が低いものは若手1人で行い、難易度が高いものやデータベース設計はベテラン2人で行います。このコードレビューを行う人数の基準も文書として残しています。
4. チーム内コミュニケーション
アジャイル開発では、常にチームメンバー間でコミュニケーションを取りながら開発を行います。そのため、コードレビューは開発者のスキル向上や、開発者間のコミュニケーションの場として有用です。
基本的にはレビュー内容を後で確認できるように、GitHubにコメントとして残しています。しかし残すまでもない細かい議論はSlackを使って、合意した結果だけ残しています。
その一方で、大きな修正を依頼するときや、自分でも迷っているときなど、文字では伝わりにくいと感じたときは、Google Meetを使って、ときには画面を共有して口頭で認識合わせや方向性の確認をしています。
おわりに
自分はメンバーズエッジカンパニーに入るまで、コードレビューをしたことも、受けたこともありませんでした。仕事に裁量はありましたが、充実感が足りませんでした。
しかし今では、チーム開発で充実した日々を過ごしています。そしてその理由の一つに、コードレビューが入ることは間違いありません。まだコードレビューを取り入れていないチームも、是非取り入れてみてはどうでしょうか。
