はじめに
皆さんもコードレビューを依頼されることがあると思いますが、時間取れない時はどうしているでしょうか。
後回しでしょうか?それとも、今やっているタスクを止めてでもやっていますか?
自分は前者です。そのため、レビューがボトルネックになることが多いです。(反省)
今回は、自分がぱつっていて、どうしても時間が取れない時に、どうしてもやらないといけないコードレビューが発生した時に、外さない観点についてお伝えできればと思います!
レビュー時に見る観点
他にもあるかと思いますが、自分はだいたいこんなところを見ます。
・設計書通りに動くか
・命名規則に忠実であるか
・変数・関数のスコープは最小限にできているか
・長すぎるロジックがないか
・共通化すべきところを適切にできているか
・コメントをちゃんと書いてるか
・ロジックを適切な場所に記載できているか
全部見ようとすると結構時間がかかるのですが、レビューをすることがメインタスクではないため、後回しになりがちです。(申し訳ない気持ちで押しつぶされる)
忙しい時に見ているレビュー観点
返済しづらい技術負債になりがちな下記観点は見るようにしています。
・ロジックを適切な場所に記載できているか
ロジックの置き場所を間違えると、だいたい同じだけど微妙に違うコードが別で記述されてしまったり、利用してはいけない場所で利用されて思わぬところに影響範囲を持ってしまった可哀想な子が生まれたりと、変更する心理的ハードルが高いコードが誕生してしまいます。
ロジックを適切な場所に記載するために必要な考え方
よくRoRのレビューする機会があり、MVCの観点でどこにビジネスロジックを記述するのが適切かという話題になるのですが、基本Modelだよねという話は皆さん共通の認識かと思います。それでは、「どのModelに記述して、どう呼び出すべきか」という観点になると、どうでしょうか。経験上、答えはばらけることが多いです。
私がその際に利用している考え方は、ドメイン駆動設計の集約です。
ドメイン駆動設計の集約とは
ドメイン駆動設計の集約 (Aggregate) とは、関連するオブジェクト群を 1 つのユニットとして管理するための手法です。集約は、1 つのルートエンティティ(集約ルート)と、それに関連するエンティティや値オブジェクトで構成されます。
その集約を設計に適用する考え方については、こちらの記事がわかりやすかったです。(ドメイン駆動設計の集約のわかりにくさの原因と集約を理解するためのヒント)
上記の記事で「集約の設計は、ロジックの置き場所の設計」と記載がある通り、集約はロジックを適切な場所に記載するための設計をするのに重要な考え方です。
実際に集約をレビュー観点に落とし込むとどういったことを見るか
具体的には下記項目となります。そして、1・2・3に関しては設計段階でレビューをすることが多いです。ここの設計がある程度正しく、チームのメンバー間で認識が擦り合っていれば、ほぼなんとかなります。
また、PR時のレビューもそんなに時間がかかりません。
- 現実とリンクするように、意味のあるクラスの集合体(集約)を適切に設計できているか
- 集合体の各要素に適切にメソッドを生やせているか
- それぞれの要素のメソッド名が意味のある言葉になっているか
- 集合体(集約)に含まれる要素を更新する際に、代表クラス(集約ルート)のメソッドの呼び出しでのみ、変更できるようになっているか
- デメテルの法則に著しく違反していないか
総括
生成AIでこの辺りも自動でレビューしてくれるようになると捗りますね。
レビューを短時間で行えると開発の生産性も向上するというデータもあるので、忙しくても、レビューはすぐできるような文化づくりをしていきましょう!(自戒)