スマレジの テックファーム(SES 部門) でWebエンジニアとして働いている やまて(@r_yamate) と申します。
実務では、2023 年 3 月末で SES の派遣先で、テーブルオーダーシステムの機能改修業務の設計などを担当していた業務を終えたところです。
4月からは、スマレジの関連アプリの開発業務を担当しています。触ったことのなかった Flutter での開発で、日々奮闘中です。
はじめに
一週間ほど前からレビュアーとして、コードレビューデビューしました。今回の記事では、コードレビューはどうやったら上手くできるか、という課題とじっくり向き合うことにします。
現状の課題
コードレビューについての、現在の自分の課題は以下のとおりです。
コードレビューのチェック観点を理解していない
正直なところ、これまで自分が出したGitLabでのマージリクエストに対するレビューでコメントいただいた内容と比較したら、レビュアーとしての自分のコードレビューは、(コードをただ眺めてるだけなのでは…)(表面的なレビューしかできていない…)という感覚で、このままではいけないと思っています。
経験不足ゆえに、自分のレビューに自信を持てない
ここはより良い書き方がありそう、と思ったことはなるべく伝えるように心がけていますが、「こういう意図なんですけど変更する必要ありますか?」という返しがあると、自信が持てなくなります。
ただ単に技術力が不足している、という点は今すぐにはどうにもできないため、一旦技術力不足は置いておいて、現状の自分にできる最善は尽くす方法をじっくり考えてみることにします。
目次
何はともあれ、ChatGPT に質問
まずは ChatGPT(4.0)に質問です。
Q1. コードレビューとは何か
レビューは受けてきたため、フローはわかるのですが誤解がないか一応確認します。
Q1. 業務で初めて、レビュアーとしてコードレビューをすることになりました。コードレビューとは何か、基本をまずは教えてください。
A1.
コードレビューは、プログラミングにおける開発プロセスの一部であり、一人または複数人の開発者(レビュアー)が他の開発者(オーサー)のコードを検討し、エラー、バグ、改善の可能性を探す活動のことを指します。目的は、コードの品質を向上させ、一貫性を保つことであり、より大きなシステムへのエラーの波及を防ぎます。
コードレビューの基本的な手順は次のとおりです:
準備
オーサーがレビュアーにコードを提出します。このコードは通常、バージョン管理システムにコミットされ、プルリクエストとして送信されます。
レビュー
レビュアーは提出されたコードを検討します。ここでレビュアーが考慮するべき点は、コードの機能性、効率性、繰り返しの最小化(DRY原則)、可読性、コメントの適切性、テストの有無や品質などです。
フィードバック
レビュアーはフィードバックを提供します。これは、明確なバグの指摘、コードの改善提案、理解を深めるための質問など、様々な形式をとります。
修正
オーサーはレビュアーのフィードバックに基づいてコードを修正し、再度レビューを求めます。このプロセスは、全ての問題が解決されるまで繰り返されます。
承認とマージ
最終的にレビュアーが提出されたコードを承認し、そのコードは元のコードベースにマージされます。
効果的なコードレビューを行うためには、尊重と建設的なコミュニケーションが重要です。レビュアーは、自分の視点と経験を共有することでオーサーの成長を支援するとともに、自分自身も学びを深めることができます。また、レビューは単にバグを見つけ出すだけでなく、チーム全体のコーディング規範の統一やベストプラクティスの共有、コードベースの理解を深めるための機会でもあります。
ChatGPT の回答 A1. への感想
レビュアーは、自分の視点と経験を共有することでオーサーの成長を支援するとともに、自分自身も学びを深めることができます。
という一文がとても刺さりました。
正直なところ、自分自身の経験が浅く、「自分の経験を共有する」ということが私にはほぼできません。
そのため、レビュアーとしての責務を果たすべく少しでもまともなレビューをしようと思ったら、自分自身が追加や変更などの意図を理解して、その上で適切な観点でコードを読み込んで、適切な観点でチェックする必要があります。
その過程を私自身の成長の場としても捉えて、自分なりの少しでも良いレビューをできるようにしたいと思いました。
Q2. レビューでレビュアーが考慮するべき点
Q2. レビューでレビュアーが考慮するべき点を詳細に教えてください。
A2.
レビュアーが考慮すべき主な要素は以下の通りです:
- コードの機能性
- コードは、それが設計された仕様を満たしているか?
- それは正しく動作するか?
- 効率性
- コードは効率的に実行されるか?不必要にリソースを消費していないか?
- また、適切なデータ構造とアルゴリズムが使用されているか?
- DRY原則(Don't Repeat Yourself)
- コード内で重複している部分はないか?
- 共通の機能は適切に抽象化され、再利用可能な関数またはクラスとして組織化されているか?
- 可読性
- コードは一貫性があり、簡潔で、理解しやすいか?
- 適切なインデントやスペーシング、命名規則が適用されているか?
- コメントの適切性
- コードの目的、複雑な部分、重要な決定を説明するコメントは適切に記載されているか?しかし、冗長なコメントまたはコード自体が明白である場合に不必要なコメントは避けるべきです。
- エラーハンドリング
- コードは潜在的なエラーや例外を適切に処理しているか?
- それらは適切にログに記録されているか?
- テスト
- テストは適切に書かれており、主要な機能やエッジケースをカバーしているか?
- コーディング規約の順守
- コードはチームまたはプロジェクトのコーディング規約やスタイルガイドに準拠しているか?
- セキュリティ
- コードは潜在的なセキュリティリスクを持たないか?例えば、SQLインジェクションやXSS攻撃に対して安全であるか。
- モジュラリティとスケーラビリティ
- コードは適切にモジュール化されており、新たな要件や将来の変更を容易に取り入れられるか?
ChatGPT の回答 A2.への感想
これはそのままチェックシートのように確認事項として考えようと思いました。
コードレビューにかけられる限られた時間で、むやみにコードと睨めっこして、運良く見つけたことをコメントするよりも、これらの主な要素をもとに観点を変えて、コードを見るようにすると良いレビューにつながりそうだと思いました。
Q3. フィードバックでレビュアーが考慮するべき点
Q3. コードレビューのフィードバックで、レビュアーが考慮するべき点を詳細に教えてください。
A3.
コードレビューのフィードバックは、コードの品質を向上させ、オーサーの技術力を高め、一貫性と理解を促進するための重要な手段です。以下に、フィードバックを提供する際にレビュアーが考慮すべき要点をいくつか挙げてみます:
具体性と明確さ
フィードバックは具体的で明確でなければなりません。問題を特定し、なぜそれが問題であるのかを説明し、可能であれば改善策を提案します。
敬意と尊重
フィードバックは常に敬意をもって提供すべきです。批判的なトーンを避け、問題を提起する際には中立的な言葉を使用します。オーサーの努力を認識し、良い部分を称賛することも忘れないようにしましょう。
コンテキストと優先順位
フィードバックはコードのコンテキストとビジネス要件を考慮に入れるべきです。全ての問題が同じ優先順位を持つわけではないため、問題の重要度を理解し、それに応じて優先順位を付けることが重要です。
教育的な視点
レビューは学びの機会です。可能であれば、なぜあるコードがベストプラクティスでないのか、どのような問題が発生する可能性があるのかを説明しましょう。
オープンマインド
自分の提案が常に最善であるとは限りません。オーサーからの反論や別の視点に開かれていることが重要です。
一貫性
レビューの一貫性も重要です。同じ種類の問題に対して一貫したフィードバックを提供し、プロジェクト全体で一貫したコーディングスタイルと規約を保つようにします。
レビューは批判ではなく、チームとして最善のソリューションを見つけるためのコラボレーションであるべきです。以上の要点を考慮に入れることで、レビュアーはより効果的なフィードバックを提供することができるでしょう。
ChatGPT の回答 A3. への感想
全て「基本の基」と感じられる要点です。
オーサーの努力を認識し、良い部分を称賛することも忘れないようにしましょう。
という点については、称賛をコメントとして伝えるのはかなり意識的にしないとできない行動だと思いますが、良いと思った点を見つけた場合は伝えるように心がけたいです。
コードレビューについての記事の検索
続いて、コードレビューについての優良記事を探してみます。
以下の 3 つの施策を提唱していて、読むことでより具体的な行動のイメージが湧きました。
3つ施策
- 関数単位でOK,NGを記録する
- 四回に分けてレビューする
- リジェクト基準を作る
自分以外の人を巻き込む必要のある施策は参考程度に、まずは自分がコードレビューするときの参考にします。
プログラマとして成熟しているレビュアーが新人プログラマにレビューする場合の、悪いレビュー → 良いレビューをまとめてあって、とても参考になりました。「公式ドキュメントで殴る」「わからない単語を並べる」が、個人的には一番イヤでした…
読んでいて、私自身が犯しがちな過ちだなぁ…と思いながらも、レビュアーとしてはコードレビューのチェックリストにできそうな内容でした。
通読してタメになったと感じた記事です。
おわりに
今回の記事では、コードレビューはどうやったら上手くできるか、という課題とじっくり向き合いました。
経験不足で自分のレビューに自信を持てなくても…
色々と読んで、コードレビューは必ずしも熟練者のような振る舞いをする必要はないとも感じました。経験の浅い自分のレビューに自信を持てない、という現状の課題を冒頭で書きましたが、今すぐには実力以上のレビューはできないので、これは考えても仕方ないと思いました。
今回インプットしたことをこれからの行動に反映させて、今後もコードレビューはどうやったら上手くできるか、という課題と向き合っていきます。おすすめの記事があったら教えていただけるとありがたいです。
ありがとうございました。
案件入って3ヶ月、エラー解決で丸一日がかりで詰まり倒したときに原因だったのは、いつも基礎的なことの理解不足でした。
— やまて|ソフトウェアエンジニア2年目 (@r_yamate) 2023年7月2日
・固定IPアドレスの重複
・Wi-FiのIP設定をstaticにした時のDNSの設定
・HTTP ヘッダー名の大文字小文字は区別されない
基礎の理解の浅さで詰まってる印象なので、もっと知識を深めたい!...