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

by kotauchisunsun

Slide 1

Slide 1 text

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

Slide 2

Slide 2 text

自己紹介　～仕事編～ ● @kotauchisunsun ● 株式会社STYLY 2019年入社 ● サーバーサイド責任者 ● プラットフォーム部 EM

Slide 3

Slide 3 text

自己紹介　～個人編～

Slide 4

Slide 4 text

今日お話しする話の元ネタ

Slide 5

Slide 5 text

サーバーサイドのリリース回数が年間418回を達成最大は１か月80回！！

Slide 6

Slide 6 text

サーバーサイドのソースコードの自動テストのカバレッジ (Googleの平均テストカバレッジは79%らしい・・・・)

Slide 7

Slide 7 text

当時の開発体制委託委託企画・設計開発レビュー

Slide 8

Slide 8 text

当時の開発体制委託委託企画・設計開発レビュー PRレビューが全部私に来る

Slide 9

Slide 9 text

年間400件超のコードレビューを頑張った話

Slide 10

Slide 10 text

コードレビューの効率をあげたい

Slide 11

Slide 11 text

コードの見る行数を減らそう

Slide 12

Slide 12 text

コードレビューの着眼点仕様とプログラムが合致しているかそれ以外 ● それ以外 ○ タイポしている ○ １行が長すぎる ○ １クラスが大きすぎる ○ 関数の引数が多すぎる ○ 非推奨の関数を使っている ○ コードのにおい ○ バッドプラクティス「それ以外」を見る工数を限りなく削減しよう

Slide 13

Slide 13 text

Linter/静的解析の導入 PHP-CS-Fixer PSR-2(コード規約チェック） PHPMetrics LCOM・循環複雑度 SonarQube コードのにおいの検出複数の視点からコードレビューの自動化を行った

Slide 14

Slide 14 text

コードレビューの効率はあがった

Slide 15

Slide 15 text

なぜか？

Slide 16

Slide 16 text

コードレビューは難しい

Slide 17

Slide 17 text

コード原則１原則2 原則3 ・・・原則N ②自分の脳内の原理原則一覧を列挙し、 ③該当する原則を思い出し、 ④その原則の妥当性を示し、 ①原則に違反してそうな部分を見つけ、 ⑥修正方針を示す “仕様と実装の一致以外”ー≒”コードの良さ”のコードレビュー結構大変。レビュー ⑤コードが原則に違反している　ことを示し、例：動詞から始まらない関数の命名

Slide 18

Slide 18 text

SonarQube(SonarCloud)の場合 ①どのようなコードのにおいか？ ②ソースのどの箇所で発生してるか？ ③どのように修正すべきか？ SonarQubeがレビューのフローをほぼ自動でやってくれる

Slide 19

Slide 19 text

コードレビューで考えることが少なくなった

Slide 20

Slide 20 text

コードレビューの効率はあがった

Slide 21

Slide 21 text

それでもうまくいかない部分があった

Slide 22

Slide 22 text

WebApiInterface コードの一貫性の破れ UserController PostController FavoriteHandler MessageController ほかのクラス群はControllerで統一されているのにクラスの命名がHandlerとして実装してしまう。既存のツールでは対応できない。

Slide 23

Slide 23 text

PHPのリフレクション機能プログラム内で定義されたクラスの名前が全て取得できる関数各クラスの定義情報が取得できるクラス

Slide 24

Slide 24 text

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のテストケースとして相乗り。

Slide 25

Slide 25 text

ルールの一例 ● Controllerパッケージのクラス名はControllerをsuffixとする ● Applicationパッケージのクラス名はApplicationをsuffixとする ● コンストラクタの引数には型必須 ● 関数の引数には型必須 ● 関数の返り値には型必須 ● ファイルの行数は1,000行以下 ● 作成日時(created_at)など日時をあらわす引数は日時型(Carbon)とする PHP-CS-Fixerで実現できない細かいルールも追加社内独自の一貫性も確保できるようになった

Slide 26

Slide 26 text

別に意外な副産物があった

Slide 27

Slide 27 text

今までのコードレビュー既存コード委託追加コードこのコードは既存コードのルールと逸脱しており、社内では〇〇というルールで実装しています。修正お願いします。

Slide 28

Slide 28 text

Linterを作成しての気づき委託既存コード追加コードルール1 Linter1 自分では曖昧性のないルールで運用しているつもりだが、 Linterのルールとして実装できない矛盾を含んだルールだった。ルール2 Linter2 Linterとして実装できて、自分としては守っているつもりのルールが、既存コードが守られていなかった。 ● いかに自分のコード (一貫性)に関する認識が甘かったかを痛感。 ● 論理矛盾のないルールは検証なしで設計できない。 ● 厳格で精緻な運用ができるしすべきと思うならプログラムに実装できるはずで、それなら自分で実装して運用すべき。

Slide 29

Slide 29 text

もう1つの効果

Slide 30

Slide 30 text

当初のリリースサイクル委託開発レビュー開発レビューリリースコードの品質を上げる方法がコードレビューしかない。人力コードレビューがボトルネック

Slide 31

Slide 31 text

改善後のリリースサイクル委託開発開発レビューリリース既存ツール自作Linter チェックコードの品質が上がるので・レビュー時間が短縮・差し戻し回数が削減開発からリリースまでの時間が短縮 →年間400超リリース出来たコードの品質向上が開発者で完結レビュー待ち時間の削減開発時間の増加

Slide 32

Slide 32 text

コードレビューの観点の再分解と効率化仕様とプログラムが合致しているかそれ以外既存ツールで対処自作Linterで対処一般的なプラクティス社内固有プラクティス仕様とプログラムが合致しているかツールにより見るべきソースコードが削減できた。 →大量のレビューができるようになった。

Slide 33

Slide 33 text

直感的には正しそう。

Slide 34

Slide 34 text

本当？

Slide 35

Slide 35 text

別視点から整理してみたい

Slide 36

Slide 36 text

18章近年のコードレビュージェイソン・コーエン(Jason Cohen)

Slide 37

Slide 37 text

コードレビュー時間と欠陥の発見数の関係（引用）ところが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. 横軸：レビュー時間縦軸：欠陥数

Slide 38

Slide 38 text

レビュー速度と欠陥密度の関係（引用）問題は、400～500LOC/時を超える早いレビューにあります。突然、欠陥密度の高いレビューは稀になります。(中略) レビュー担当者が速く進めすぎて欠陥を見つけ損なっているからです。速すぎるコードレビューはバグを見逃すので、レビュー速度は400行/時未満にすべき Cohen, Jason. 2006. Best Kept Secrets of Peer Code Review. Baberly, MA: SmartBear Software 横軸：レビュー速度(LOC/時) 縦軸：欠陥密度(欠陥数/kLOC)

Slide 39

Slide 39 text

コードレビューの限界（引用）レビューは最大で1時間にすべきであり、(中略) 1回のレビュー対象は400行を超えてはいけないという結論になります。 ● 1回のコードレビューは1時間まで ● 1回のコードレビューは400行まで人間のコードレビューの量に限界がある。横軸：レビュー対象の行数(LOC) 縦軸：欠陥密度(欠陥数/レビュー時間)

Slide 40

Slide 40 text

既存ツールの導入や自作ツールの開発によりコードレビュー量は増やせる。人間のコードレビューには限界がある。 1回1時間、400行までしかコードレビューができない。矛盾？

Slide 41

Slide 41 text

ちょっと別の話

Slide 42

Slide 42 text

認知負荷理論参考：https://atlassian-teambook.jp/_ct/17574839 オーストラリアの教育心理学者、ジョン・スウェラー（John Sweller）氏によって1980年代に開発・確立された理論脳のワーキングメモリが常に処理している「情報量」を指し、脳への情報のインプットが私たちの認知能力にどのような負担をかけるかを表している。

Slide 43

Slide 43 text

認知負荷の3つのタイプ ● 課題内在性負荷(Intrinsic Cognitive Load) ○ 情報の内容を理解するのが困難な度合い ● 課題外在性負荷(Extraneous Cognitive Load) ○ 気を散らせる情報や事象といった外的要因によって引き起こされる認知の負荷 ● 学習関連負荷(Germane Cognitive Load) ○ 脳がスキーム(世界を理解するための枠組み)を構築しているときに発生する。スキームの構築・蓄積により、同様な状況で適切な判断ができるようになる。参考：https://atlassian-teambook.jp/_ct/17574839

Slide 44

Slide 44 text

ダンスを学ぶ時の認知負荷の解釈音楽に合わせて、腕を振り上げて・・・・右足をあげて、右手をにぎって、上体を反らして・・・ダンスを学ぶためにスマホで検索して・・・課題内在性負荷学習関連負荷課題外在性負荷 ※発表者は専門家ではありません

Slide 45

Slide 45 text

コードレビューの認知資源の内訳課題内在性負荷課題外在性負荷人間の耐えられる認知負荷の限界課題外在性負荷課題内在性負荷余力課題外在性負荷を減らすことができれば、レビュー量が増やせる？

Slide 46

Slide 46 text

コードレビュー効率化の実践と認知負荷理論の接合仕様とプログラムが合致しているか (課題内在性負荷) それ以外 (課題外在性負荷) 一般的なプラクティス社内固有プラクティス仕様とプログラムが合致しているか (課題内在性負荷) 人間の耐えられる認知負荷の限界 = 人間のコードレビューできる限界コードレビューできる規模が増えた自作・既存ツールで節約した情報量 MakingSoftwareの結論と認知負荷理論に矛盾することなく今回の手法と成果を整理できた。 →今回のレビュー効率化の手法はおそらく有効そう？

Slide 47

Slide 47 text

まとめ ● 2022年にコードレビューが急増した ● コードレビューの効率化を行った ○ 既存ツールの導入・自作Linterの開発 ● 自作Linterの開発で自分のコードに関する認識を改めた ○ 自分のコード規約の矛盾の認知・一貫性の欠如 ● コードレビューの改善によりリリースサイクルが短縮化された ● コードレビューは1時間の間に、1回400行未満にしないとバグを見逃す危険性が高まる。 ● 認知負荷には3種類ある ○ 課題内在性負荷・課題外在性負荷・学習関連負荷 ● 大量のコードレビューが可能になった理由は、既存ツール・自作 Linterの開発により課題外在性負荷を軽減することで、仕様と実装の不一致の確認に認知資源を注力できたことによるものだと整理できた ● 課題外在性負荷の軽減により年間400超のリリースが実現できた