この記事はWHITEPLUS Advent Calendar 2021の6日目の記事です。
はじめに
こんにちは、ホワイトプラスのバックエンドチームでEMをしているinouehiです。
ホワイトプラスではコードレビューが活発に行われています。
コードレビューは品質の向上、技術の伝承、業務知識や商品知識の伝承に役立っており、この文化・習慣を大事にしたいと考えています。
レビューすること自体は当たり前となっていますが、その一方で、レビューの観点がわからないなどレビューに対して難しさを感じる声を聞くこともあるため、いくつかの観点を言語化してみました。
レビューの準備
レビュワーは実現したいことを理解してレビューに臨む。そのため、要件定義、モデリング成果物やテストコードを参考にする。（レビュイーはこれらを作成することが求められる。）
レビュイーは、レビューしやすいように配慮する。例えば、
- 前提知識が必要な部分、複雑な部分や実装のキモとなる部分に対してコメントをつけておいたり、レビューの前に口頭で説明したり、認識合わせを行う。
- 適度にコミットを分ける。
レビュー観点例
- 要件（機能要件・非機能要件）を満たしているか
- 期待通りに使えるか
- 使いやすいか
- リーダブルか
- 無駄なコードがないか（YAGNIに反してないか）
- 概念の重複がないか（DRYに反していないか）
- 効率的か
- パフォーマンスに問題はないか
- 無駄にリソースを消費してないか
- リソースの増大を考慮しているか
- セキュリティの問題はないか
- 障害の原因が特定しやすいか
- 障害からの復旧が容易か
- デグレードを起こしていないか
- モデリングは妥当か
- モデリングがコードに正しく反映されているか
- コーディング標準に沿っているか
- もっと簡潔に書けないか
- テストが書かれているか
- テストケースに過不足ないか
- 仕様書としてわかりやすいか
- PRのタイトル、概要説明、添付資料、レビュワー、マージ先、ブランチ名は適切か
- 動作確認の観点は十分か
※必ずしもMECEじゃないけどご容赦下さいmm
補足
要件（機能要件・非機能要件）を満たしているか
- 品質特性が参考になる。（『つながる世界の ソフトウェア品質ガイド - IPA』（PDF））
- CRUDに不足はないか、データがない場合や境界値の挙動に不備はないか、状態遷移に過不足はないかのように、ある程度普遍的な観点でチェックする。
- リーダブルに含まれる観点は膨大。『リーダブルコード』などを参考に。
- セキュリティに含まれる観点も膨大『安全なウェブサイトの作り方』などを参考に。
モデリングは妥当か
- 高凝集・低結合か、アーキテクチャは適切か、ユビキタス言語（ステークホルダが共通認識をもてるドメインを表現する言葉）で書かれているかなど。
- DDD、オブジェクト指向、関数型プログラミングなどのプログラミングパラダイムに関する理解を深める必要がある。
コーディング標準に沿っているか
静的解析でチェックするが、人手でチェックする部分もある。
もっと簡潔に書けないか
簡潔な構文で書く、言語の標準関数やフレームワークの機能を利用する（車輪の再発明をしない）など。
その他
- 読みにくいと感じた部分は改善のシグナル。
- レビュワーは評価者ではなくContributor。
- レビュワーもコードから学び、学びがあったら感謝を伝える。
まとめ
レビュー観点はケースバイケースであるため、具体的な観点のチェックリストのようなものを提示することはできませんが、レビューに取り組むとっかかりを見つけるきっかけにでもなれば幸いです。
さいごに
ホワイトプラスでは、ビジョンやバリューに共感していただけるエンジニアを募集しています！
ネットクリーニングの「リネット」など「生活領域×テクノロジー」で事業を展開している会社です。どんな会社か気になった方はオウンドメディア「ホワプラSTYLE」をぜひご覧ください。
オンラインでカジュアル面談もできますので、ぜひお気軽にお問い合わせください。
