こんにちは。決済システムでエンジニアをやっております hoshino33 です。
今回はコードレビューのレビュアーになった際にどういった観点でレビューしているか書いていこうと思います。
環境について
決済システムでの開発においては以下を利用しております。
- 開発言語：C#
- IDE：Microsoft Visual Studio + ReSharper もしくは JetBrains Rider
- ユニットテスト：MSTest
- Gitのホスティングサービス：Bitbucket
はじめに
コードレビューを依頼する際はプルリクに以下を記載するようにしています。
要件定義
実装対象の要件定義を記載します。ConfluenceもしくはNotionで記載したリンク。
セキュアコードレビューシート
コードレビューとは別にセキュアコードレビューシートを用意しコードレビューと同時にセキュアコードレビューを行います。詳しくはこちらをご参照ください
実装内容
実装内容の説明
画面プレビュー
画面がある場合はスクショ
ユニットテストの実行結果
ユニットテストの実行結果のスクショ
コードレビューの観点
前置きが長くなりましたが、今回のメインテーマになります。
要件定義の内容を実装しているかどうか
こちらは言わずもがな。
コーディング規約が守られているか
弊社の場合は、コーディング規約がいくつかありますのでそちらに従っているかどうか。
typoがないか
スペルミスがないかどうか。 ちなみに私はコーディングに限らずtypoが多め。。
命名は適切か
一般的に使用されている命名かどうか。
あとは、スコープが広いものについては多少長くなっても良いのでわかりやすいかどうか。狭い場合であればある程度許容します。
トリッキーな実装をしていないか
無理やり記述するコード量を少なくするなど難解なコードが書かれている場合。チーム開発においては難解なコードより他人にわかりやすいコードの方が有益だと思います。
すでにある機能を再実装していないか
すでにある機能（メソッドなど）がある場合はそちらを使用してもらう。
条件が複雑すぎないか
条件が複雑すぎるとどういった条件かわからなくなるため、分解してもらうなど簡易化してもらう。
不要なコード
デッドコードや不要な変数、メソッドがある場合は削除してもらう。
適切な場所に記載されているかどうか
適切なクラスにロジックがかかれているかなど。
コメントが適切かどうか
基本はドキュメンテーション コメントになりますが、処理中にコメントがある際は、有益な情報かどうかを確認します。
ネストが深すぎないか
ネストが深く、可読性をさげていないかどうか。
エラーハンドリング
エラーハンドリングが適切かどうか。不必要に握り潰していないかどうかなど。
まとめ
みなさんはどのような観点でレビューしていますか？
他にも観点はありそうな気がしますが大体似たような観点になってくると思いますが、人によって観点が微妙に違ってくるので、この観点を意思統一することができればさらに品質を上げることができると思います。
また、ツール類に頼れる部分は任せて、人でしか確認できない部分を中心にレビューができる環境を作っていけるとなお良いと思います。
正直、レビューは結構体力を使いますが、レビューの中で新たな発見があったときの学びは嬉しいですね。
We are hiring！！
ROBOT PAYMENTでは一緒に働く仲間を募集しています!!!
speakerdeck.com
www.robotpayment.co.jp
🎉twitter採用担当アカウント開設！🎉どんどん情報発信していきます！！
