Upgrade to Pro — share decks privately, control downloads, hide ads and more …

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

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

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

kotauchisunsun

May 11, 2024
Tweet

More Decks by kotauchisunsun

Other Decks in Programming

Transcript

  1. コードレビューの着眼点 仕様とプログラムが 合致しているか それ以外 • それ以外 ◦ タイポしている ◦ 1行が長すぎる

    ◦ 1クラスが大きすぎる ◦ 関数の引数が多すぎる ◦ 非推奨の関数を使っている ◦ コードのにおい ◦ バッドプラクティス 「それ以外」を見る工数を限りなく削減しよう
  2. コード 原則1 原則2 原則3 ・・・ 原則N ②自分の脳内の原理原則一覧を列挙し、 ③該当する原則を思い出し、 ④その原則の妥当性を示し、 ①原則に違反してそうな部分を見つけ、

    ⑥修正方針を示す “仕様と実装の一致以外”ー≒”コードの良さ”のコードレビュー 結構大変。 レビュー ⑤コードが原則に違反している こ とを示し、 例:動詞から始まらない関数の命名
  3. 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のテストケースとして相乗り。
  4. ルールの一例 • Controllerパッケージのクラス名はControllerをsuffixとする • Applicationパッケージのクラス名はApplicationをsuffixとする • コンストラクタの引数には型必須 • 関数の引数には型必須 •

    関数の返り値には型必須 • ファイルの行数は1,000行以下 • 作成日時(created_at)など日時をあらわす引数は日時型(Carbon)とする PHP-CS-Fixerで実現できない細かいルールも追加 社内独自の一貫性も確保できるようになった
  5. Linterを作成しての気づき 委託 既存コード 追加コード ルール1 Linter1 自分では曖昧性のないルールで運用しているつもりだが、 Linterのルール として実装できない矛盾を含んだルールだった。 ルール2

    Linter2 Linterとして実装できて、自分としては守っているつもりのルール が、既存コードが守られていなかった。 • いかに自分のコード (一貫性)に関する認識が甘かったかを痛感。 • 論理矛盾のないルールは検証なしで設計できない。 • 厳格で精緻な運用ができるしすべきと思うならプログラムに実装できるはずで、 それなら自分で実装して運用すべき。
  6. 改善後のリリースサイクル 委託 開発 開発 レビュー リリース 既存ツール 自作Linter チェック コードの品質が上がるので

    ・レビュー時間が短縮 ・差し戻し回数が削減 開発からリリースまでの時間が短縮 →年間400超リリース出来た コードの品質向上が開発者で完結 レビュー待ち時間の削減 開発時間の増加
  7. 認知負荷の3つのタイプ • 課題内在性負荷(Intrinsic Cognitive Load) ◦ 情報の内容を理解するのが困難な度合い • 課題外在性負荷(Extraneous Cognitive

    Load) ◦ 気を散らせる情報や事象といった外的要因によって引き起こされる認知の 負荷 • 学習関連負荷(Germane Cognitive Load) ◦ 脳がスキーム(世界を理解するための枠組み)を構築しているときに発生す る。スキームの構築・蓄積により、同様な状況で適切な判断ができるように なる。 参考:https://atlassian-teambook.jp/_ct/17574839
  8. コードレビューの認知資源の内訳 課題 内在性 負荷 課題 外在性 負荷 人間の耐えられる認知負荷の限界 課題 外在性

    負荷 課題 内在性 負荷 余力 課題外在性負荷を 減らすことができれば、レ ビュー量が増やせる?
  9. コードレビュー効率化の実践と認知負荷理論の接合 仕様とプログラムが 合致しているか (課題内在性負荷) それ以外 (課題外在性負荷) 一般的なプラクティス 社内固有プラクティス 仕様とプログラムが 合致しているか

    (課題内在性負荷) 人間の耐えられる認知負荷の限界 = 人間のコードレビューできる限界 コードレビューできる規模が増えた 自作・既存ツールで 節約した情報量 MakingSoftwareの結論と認知負荷理論に矛盾することなく今回の手法と成果を整理できた。 →今回のレビュー効率化の手法はおそらく有効そう?
  10. まとめ • 2022年にコードレビューが急増した • コードレビューの効率化を行った ◦ 既存ツールの導入・自作Linterの開発 • 自作Linterの開発で自分のコードに関する認識を改めた ◦

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