ソースコードを美しくたもつために ～コードレビューの認知限界を突破し、年間400リリースを達成する～

2024/05/11に開催されたPHPカンファレンス香川にて発表
https://phpcon.kagawa.jp/2024/

Transcript

1. ソースコードを美しくたもつために ～コードレビューの認知限界を突破し、年間400リリースを達成する～ @kotauchisunsun
2. 自己紹介　～仕事編～ • @kotauchisunsun • 株式会社STYLY 2019年入社 • サーバーサイド責任者 • プラットフォーム部 EM
3. 自己紹介　～個人編～
4. 今日お話しする話の元ネタ
5. サーバーサイドのリリース回数が年間418回を達成 最大は１か月80回！！
6. サーバーサイドのソースコードの自動テストのカバレッジ (Googleの平均テストカバレッジは79%らしい・・・・)
7. 当時の開発体制 委託 委託 企画・設計 開発 レビュー
8. 当時の開発体制 委託 委託 企画・設計 開発 レビュー PRレビューが全部私に来る
9. 年間400件超のコードレビューを頑張った話
10. コードレビューの効率をあげたい
11. コードの見る行数を減らそう
12. コードレビューの着眼点 仕様とプログラムが 合致しているか それ以外 • それ以外 ◦ タイポしている ◦ １行が長すぎる ◦ １クラスが大きすぎる ◦ 関数の引数が多すぎる ◦ 非推奨の関数を使っている ◦ コードのにおい ◦ バッドプラクティス 「それ以外」を見る工数を限りなく削減しよう
13. Linter/静的解析の導入 PHP-CS-Fixer PSR-2(コード規約チェック） PHPMetrics LCOM・循環複雑度 SonarQube コードのにおいの検出 複数の視点からコードレビューの自動化を行った
14. コードレビューの効率はあがった
15. なぜか？
16. コードレビューは難しい
17. コード 原則１ 原則2 原則3 ・・・ 原則N ②自分の脳内の原理原則一覧を列挙し、 ③該当する原則を思い出し、 ④その原則の妥当性を示し、 ①原則に違反してそうな部分を見つけ、 ⑥修正方針を示す “仕様と実装の一致以外”ー≒”コードの良さ”のコードレビュー 結構大変。 レビュー ⑤コードが原則に違反している　こ とを示し、 例：動詞から始まらない関数の命名
18. SonarQube(SonarCloud)の場合 ①どのようなコードのにおいか？ ②ソースのどの箇所で発生してるか？ ③どのように修正すべきか？ SonarQubeがレビューのフローを ほぼ自動でやってくれる
19. コードレビューで考えることが少なくなった
20. コードレビューの効率はあがった
21. それでもうまくいかない部分があった
22. WebApiInterface コードの一貫性の破れ UserController PostController FavoriteHandler MessageController ほかのクラス群はControllerで統一されて いるのにクラスの命名がHandlerとして実 装してしまう。 既存のツールでは対応できない。
23. PHPのリフレクション機能 プログラム内で定義されたクラスの名前が全て 取得できる関数 各クラスの定義情報が取得できるクラス
24. ReflectionClass /* プロパティ */ public string $name; //クラス名 /* メソッド */ public getAttributes(?string $name = null, int $flags = 0): array public getConstants(?int $filter = null): array public getDocComment(): string|false public getExtensionName (): string|false public getFileName(): string|false //ファイル名 public getInterfaceNames (): array //インターフェース一覧 public getMethods(?int $filter = null): array //メソッド一覧 定義済みのクラスの情報がほぼ全て取れる。 get_declared_classes+ReflectionClassで社内事情にあった簡易Linterの作成。 簡易LinterによるチェックはPHPUnitのテストケースとして相乗り。
25. ルールの一例 • Controllerパッケージのクラス名はControllerをsuffixとする • Applicationパッケージのクラス名はApplicationをsuffixとする • コンストラクタの引数には型必須 • 関数の引数には型必須 • 関数の返り値には型必須 • ファイルの行数は1,000行以下 • 作成日時(created_at)など日時をあらわす引数は日時型(Carbon)とする PHP-CS-Fixerで実現できない細かいルールも追加 社内独自の一貫性も確保できるようになった
26. 別に意外な副産物があった
27. 今までのコードレビュー 既存コード 委託 追加コード このコードは既存コードのルー ルと逸脱しており、 社内では〇〇という ルールで実装しています。 修正お願いします。
28. Linterを作成しての気づき 委託 既存コード 追加コード ルール1 Linter1 自分では曖昧性のないルールで運用しているつもりだが、 Linterのルール として実装できない矛盾を含んだルールだった。 ルール2 Linter2 Linterとして実装できて、自分としては守っているつもりのルール が、既存コードが守られていなかった。 • いかに自分のコード (一貫性)に関する認識が甘かったかを痛感。 • 論理矛盾のないルールは検証なしで設計できない。 • 厳格で精緻な運用ができるしすべきと思うならプログラムに実装できるはずで、 それなら自分で実装して運用すべき。
29. もう1つの効果
30. 当初のリリースサイクル 委託 開発 レビュー 開発 レビュー リリース コードの品質を上げる方法が コードレビューしかない。 人力コードレビューがボトルネック
31. 改善後のリリースサイクル 委託 開発 開発 レビュー リリース 既存ツール 自作Linter チェック コードの品質が上がるので ・レビュー時間が短縮 ・差し戻し回数が削減 開発からリリースまでの時間が短縮 →年間400超リリース出来た コードの品質向上が開発者で完結 レビュー待ち時間の削減 開発時間の増加
32. コードレビューの観点の再分解と効率化 仕様とプログラムが 合致しているか それ以外 既存ツールで対処 自作Linterで対処 一般的なプラクティス 社内固有プラクティス 仕様とプログラムが 合致しているか ツールにより見るべきソースコードが削減できた。 →大量のレビューができるようになった。
33. 直感的には正しそう。
34. 本当？
35. 別視点から整理してみたい
36. 18章 近年のコードレビュー ジェイソン・コーエン(Jason Cohen)
37. コードレビュー時間と欠陥の発見数の関係 （引用） ところが60分後あたりで、 突然の落ち込みがあります。(中略) 欠陥発見の効率は急激に落ち込みます。 長時間のコードレビュー疲労してバグを見逃すので、 1回のコードレビューは 1時間まで Dunsmore, A., M.Roper, and M. Wood. 2000. Object-Oriented Inspection in the Face of Delocalisation. Proceedings of the 22nd International Conference on Software Engineering 2000: 467-476. 横軸：レビュー時間 縦軸：欠陥数
38. レビュー速度と欠陥密度の関係 （引用） 問題は、400～500LOC/時を超える 早いレビューにあります。 突然、欠陥密度の高いレビューは 稀になります。(中略) レビュー担当者が速く進めすぎて 欠陥を見つけ損なっているからです。 速すぎるコードレビューはバグを見逃すので、 レビュー速度は400行/時未満にすべき Cohen, Jason. 2006. Best Kept Secrets of Peer Code Review. Baberly, MA: SmartBear Software 横軸：レビュー速度(LOC/時) 縦軸：欠陥密度(欠陥数/kLOC)
39. コードレビューの限界 （引用） レビューは最大で1時間にすべきであり、(中略) 1回 のレビュー対象は400行を超えてはいけないという結 論になります。 • 1回のコードレビューは1時間まで • 1回のコードレビューは400行まで 人間のコードレビューの量に限界がある。 横軸：レビュー対象の行数(LOC) 縦軸：欠陥密度(欠陥数/レビュー時間)
40. 既存ツールの導入や 自作ツールの開発により コードレビュー量は増やせる。 人間のコードレビューには 限界がある。 1回1時間、400行までしか コードレビューができない。 矛盾？
41. ちょっと別の話
42. 認知負荷理論 参考：https://atlassian-teambook.jp/_ct/17574839 オーストラリアの教育心理学者、ジョン・スウェラー（John Sweller）氏によって1980年代に開発・確立された理論 脳のワーキングメモリが常に処理している「情報量」を指 し、脳への情報のインプットが私たちの認知能力にどのよう な負担をかけるかを表している。
43. 認知負荷の3つのタイプ • 課題内在性負荷(Intrinsic Cognitive Load) ◦ 情報の内容を理解するのが困難な度合い • 課題外在性負荷(Extraneous Cognitive Load) ◦ 気を散らせる情報や事象といった外的要因によって引き起こされる認知の 負荷 • 学習関連負荷(Germane Cognitive Load) ◦ 脳がスキーム(世界を理解するための枠組み)を構築しているときに発生す る。スキームの構築・蓄積により、同様な状況で適切な判断ができるように なる。 参考：https://atlassian-teambook.jp/_ct/17574839
44. ダンスを学ぶ時の認知負荷の解釈 音楽に合わせて、腕を振 り上げて・・・・ 右足をあげて、右手をに ぎって、上体を反らして ・・・ ダンスを学ぶために スマホで検索して・・・ 課題内在性負荷 学習関連負荷 課題外在性負荷 ※発表者は専門家ではありません
45. コードレビューの認知資源の内訳 課題 内在性 負荷 課題 外在性 負荷 人間の耐えられる認知負荷の限界 課題 外在性 負荷 課題 内在性 負荷 余力 課題外在性負荷を 減らすことができれば、レ ビュー量が増やせる？
46. コードレビュー効率化の実践と認知負荷理論の接合 仕様とプログラムが 合致しているか (課題内在性負荷) それ以外 (課題外在性負荷) 一般的なプラクティス 社内固有プラクティス 仕様とプログラムが 合致しているか (課題内在性負荷) 人間の耐えられる認知負荷の限界 = 人間のコードレビューできる限界 コードレビューできる規模が増えた 自作・既存ツールで 節約した情報量 MakingSoftwareの結論と認知負荷理論に矛盾することなく今回の手法と成果を整理できた。 →今回のレビュー効率化の手法はおそらく有効そう？
47. まとめ • 2022年にコードレビューが急増した • コードレビューの効率化を行った ◦ 既存ツールの導入・自作Linterの開発 • 自作Linterの開発で自分のコードに関する認識を改めた ◦ 自分のコード規約の矛盾の認知・一貫性の欠如 • コードレビューの改善によりリリースサイクルが短縮化された • コードレビューは1時間の間に、1回400行未満にしないとバグを見逃す危険性が高まる。 • 認知負荷には3種類ある ◦ 課題内在性負荷・課題外在性負荷・学習関連負荷 • 大量のコードレビューが可能になった理由は、既存ツール・自作 Linterの開発により課題外 在性負荷を軽減することで、仕様と実装の不一致の確認に認知資源を注力できたことによ るものだと整理できた • 課題外在性負荷の軽減により年間400超のリリースが実現できた
48. というわけで
49. みなさんCI/CDやってますか？
50. 静的解析・Linter動かしてますか？
51. 自動テスト書いてますか？
52. 千里の道も一歩から
53. がんばっていきましょう。