🐈

コードレビューって何を見れば良いの?

2022/11/19に公開約1,900字

なにこれ

筆者が、コードレビュー3年やってみて思ったことをまとめてみる!
要は備忘録というやつ。

この記事のゴール

以下をまとめられたら、コーヒータイムにしたい。

  • 現時点で良いかも?と思っているコードレビュー方針
  • レビューで工夫しているところ

筆者どのくらいレビューしている?

初めてソースコードレビューしてから3年くらい。
大体1日2〜3PRレビューしているっぽい。

本題

レビューでどのようなところ見ているか

ざっくり4つ!

  • きちんと仕様通り動くのかな
  • パフォーマンスに影響ないかな
  • 他の技術者が見て理解できるコードかな(可読性)
  • もっと良い書き方がないかな

前提: レビュー前の擦り合わせ

レビューイにどのような観点でレビューするかあらかじめ擦り合わせておく。
ルールでなく、考え方を伝えるのが良いなと思っている。
(採用しているアーキテクチャ思考など)

コーディング規約は作っても守るのがしんどいので、可能な限り自動チェックに任せてしまう。

レビュー観点1:そのコードきちんと動くの?

最初見るのは、やっぱりここ!

  • 条件分岐漏れがないか
  • 既存機能に影響がないか
  • 仕様漏れがないか

レビュー観点2: パフォーマンス影響

一度運用が始まると直しづらくなるのがDB、キャッシュ周り。

  • トラフィックの多い機能のキャッシュやDB周りに重い実装がないか
  • レコード量が多くなるであろうテーブル関連処理に重い実装がないか
  • JOINが多く複雑なSQLがないか

適切にINDEX貼ったり、DB設計見直したり、キャッシュ化してDB負荷下げたりを提案することが多いかも

レビュー観点3: 可読性が確保されているか

開発者が初見で見てある程度わかるかも?というレベルまでは確保したい。

  • 関数名とロジックが一致しているか
  • 関数は一つの役割のみを持っているか
  • ドメインが適切な粒度で分けられているか
  • ドメイン同士が密結合になっていないか
  • イレギュラーフローの場合、コメントなど説明が記載されているか

コードの理解しやすさと、変更しやすさをレビューで担保できれば開発効率が上がるかも?

レビュー観点4: もっと良い書き方の提案

お願いベースで、こんな書き方あるけどどう?と聞いてみる。
メンバーがどんな考えで書いたのかが聞けることがあるので、煙たがられない程度に提案してみる

レビュー観点のまとめ

最初は、観点ごとに1周、2周、3周、4周と周回してコードを見ていけば、レビュー漏れも少なくなるはず。
レビューイにも、喜んでもらえそう!

レビューで工夫しているところ

  • 何のためにレビューしているのかの定義
  • 機能によってレビュー粒度を変える
  • 気持ちよくレビュー依頼を出してもらう

何のためにレビューしているのかの定義

レビューってやると何となく良いし、かっこいい!と最初は思っていたのだが、きちんと目的を決めないと、「俺が考えた最強にかっこいいソースコードの書き方」を押し付けてしまうだけになるので、言語化しておいた方が良い。

レビューよりも実装した機能を適切な品質で早く提供したいので、過剰品質になってしまってもよくない。

例えば、下記な感じ

  • チームメンバー間のコード品質を、サービス運用に支障がないレベルで維持するため

機能によってレビュー粒度変える

例えば。。。

キャンペーン系など、期間限定で使用するコード: 仕様満たしてればOK(レビュー観点1のみ)

決済、ログインなど重要機能: 今後長く使うので、じっくり確認(レビュー観点全部)

気持ちよく依頼してもらう

あくまでソースコードのレビュー

命令口調ではなく、頼み事する感じでレビューコメントすると柔らかくて良い。
どのメンバーから依頼されても同じように対応する!

フォローアップを忘れずに

歴の浅い方やジョインしたてのメンバーの場合、どうしても指摘項目が増えてしまうと思う。
指摘事項が多い場合は、口頭でコミュニケーションを図ると改善する場合が多い!かも?

まとめ

普段、そんなに考えずにレビューしてしまっているが、言語化すると結構考えてるじゃん俺!となって自己肯定感が上がる↑↑! LGTMだよ!

レビューは誰でもできる工程なので、メンバーを巻き込んでコード品質をあげていけたい所存!

Discussion

ログインするとコメントできます