そもそもコードレビューとは何か
ソースコードやテストが不十分な場合、潜在的なバグやセキュリティホールなどの問題が発生しやすい。
コードには直接の不具合だけでなく、命名規則や構造設計の不備など、可読性やメンテナンス性に関する問題も存在することがある。
最適化が不足していると、リソースの無駄遣いによる性能低下が発生する可能性もある。
これらの問題を解決し、ソフトウェア品質を向上させるためには、コードレビューが重要である。
コードレビューは、開発者のスキルに依存するが、不具合や問題を発見し修正する手段の一つとして活用されている。
とWikipediaはおっしゃっています。
コードレビューの目的とは
品質向上
- クラス設計などが適切か
- 可読性、保守性は担保されているか
属人化の防止
- 他の人が理解できるコードであるか
- ソースコードはドキュメントでもあるという考え方がある
スキル向上
- 開発者の実装レベル向上させる
- 相互レビューなどをすることによりお互いの知らない知識を共有しあえる。また相乗効果なども期待できる
バグの早期発見
- チーム全体でコードを検証することで、バグや潜在的な問題を早期に発見できる
ちなみに
レビューする側を
「レビュアー(レビューア)」
レビューされる側を
「レビューイ」
といいます。(どうでもいいですね🤪)
この記事ではレビューする側を「レビュー実施者」、される側を「レビュー依頼者」と表現します。
効率の悪いコードレビューの例
レビュー形態
-
レビュー依頼者
- まず、開発を完了したレビュー依頼者がレビュー依頼を投げます。
- この時、開発したコードの差分をZipでまとめて渡したり、GitやSVNにPUSHしたので○○のコミットを確認してください。などと依頼します。
-
レビュー実施者
- 受け取ったソースを目視で順番に確認し、コーディング規約などに照らし合わせて確認する。
- 本当にレビューが必要なロジックの組み方や、他機能との整合性などの担保は厳密に規約に明文化されているわけではなく、レビュー実施者の裁量に任せられる。
フィードバック方法
- 指摘事項が見つかれば、ファイル名、行数、スクショ、内容などを1つずつExcelにまとめて、最後にレビュー依頼者に送付する。
- 指摘事項をまとめたExcelを受け取ったレビュー依頼者は、その資料を元に修正を行う。修正後に再度レビュー依頼を行う。
コードレビュー時によくある問題点や指摘点
よくある問題点
- コーディング規約レベルの指摘が多いと、本来見るべき観点に時間を割けていない
- 目視での確認がほとんどで、見落としも多い
- レビュー実施者は同じような指摘の繰り返しで時間も体力も浪費してしまう
- 既存機能からコードをコピーしてきた際に、コピー元に問題があると指摘まみれのコードが量産されてしまう
- 既存機能を修正しようにも、リリース後の機能である為、無暗に修正が出来ない
レビューでよくある指摘点
- 冗長的なコードが多い
- 条件分岐の入れ子が深い
- 不必要なループ処理が多い
- 処理コストをあまり理解せず書いている(配列やリストなどを総舐めする関数など)
- そもそもコーディング規約を守れていない
問題に対する対策と課題
コーディング規約の確認を改めて徹底する
規約の内容は最低限確認することになっているが、十分にできていない
目視での確認をしている以上、必ず見落としがある
そもそもの既存機能の問題部箇所を洗い出して修正する
修正内容によっては再テストも必要になり、良いタイミングでないとプロジェクト全体に影響を及ぼす
解析ツールや整形ツールなどの利用
導入時に手間がかかるが、導入後は負担軽減など改善が見込める
ただし、問題を多く含む既存機能に対して解析を実施した場合、大量のエラーや警告が出る可能性がある
既存機能はそのままで、新規開発分だけを修正するような事もよくある為、注意が必要
結局、コードレビューって必要？
これは「プロジェクトによる」としか答えようがありません。
しないよりした方が良いのは確実ですが、プロジェクト内容によって大きく左右されます。
コードレビューが向かないプロジェクト
たとえば、大きく遅延している状況下で尚且つ 納期のデッドラインが近いのであれば、コードレビューを実施するより、テストで動作を担保して納品する方が多くの場合において優先されるべきです。
また小規模で単発の案件や、定期的なメンテナンスを必要としないシステムなどについても、丁寧にコードレビューをするより数を回した方が良い場合が多いです。
コードレビューをした方が良いプロジェクト
パッケージ製品や自社サービス、長期間永続して運用する必要があるシステムなどにおいては、少し無理をしててでもコードレビューは実施するべきです。
コードの品質が悪いと、開発自体は無事に終わったとしても、保守や改修などにしわ寄せが来ることが多いです。
また、スケジュールが押しているからと言って、「この機能はいいや」と例外を作ってしまうと、例外が例外を呼び全体が崩れてしまいます。