Upgrade to Pro
— share decks privately, control downloads, hide ads and more …
Speaker Deck
Features
Speaker Deck
PRO
Sign in
Sign up for free
Search
Search
ソースコードを美しくたもつために ~コードレビューの認知限界を突破し、年間400リリースを達成する~
Search
kotauchisunsun
May 11, 2024
Programming
1
11k
ソースコードを美しくたもつために ~コードレビューの認知限界を突破し、年間400リリースを達成する~
2024/05/11に開催されたPHPカンファレンス香川にて発表
https://phpcon.kagawa.jp/2024/
kotauchisunsun
May 11, 2024
Tweet
Share
More Decks by kotauchisunsun
See All by kotauchisunsun
幻のLispマシン
kotauchisunsun
0
160
仮想と実存。その融合する世界を創る。 ~XR業界へ就職・転職のために必要な経験・スキルとは?~
kotauchisunsun
0
52
ARグラスにChatGPTを入れてみた V2.2
kotauchisunsun
0
140
AR グラスにChatGPTを入れてみた V2.0
kotauchisunsun
0
110
AR グラスにChatGPTを入れてみた V2.1
kotauchisunsun
1
110
ARグラスにChatGPTを入れてみた
kotauchisunsun
0
450
Apple Vision Proにみるビデオシースルー型HMDと光学式ARグラスの比較・考察
kotauchisunsun
0
2.6k
INMOAirを使い倒してみた ~ARグラスのトレードオフ仮説~
kotauchisunsun
0
6.3k
Other Decks in Programming
See All in Programming
Kotlin2でdataクラスの copyメソッドを禁止する/Data class copy function to have the same visibility as constructor
eichisanden
1
160
cXML という電子商取引の トランザクションを支える プロトコルと向きあっている話
phigasui
3
2.3k
RailsのPull requestsのレビューの時に私が考えていること
yahonda
5
2.3k
Kaigi on Rails 2024 - Rails APIモードのためのシンプルで効果的なCSRF対策 / kaigionrails-2024-csrf
corocn
5
3.6k
macOS でできる リアルタイム動画像処理
biacco42
8
2.2k
Streams APIとTCPフロー制御 / Web Streams API and TCP flow control
tasshi
2
320
推し活としてのrails new/oshikatsu_ha_iizo
sakahukamaki
3
1.8k
AWS IaCの注目アップデート 2024年10月版
konokenj
3
3.2k
Content Security Policy入門 セキュリティ設定と 違反レポートのはじめ方 / Introduction to Content Security Policy Getting Started with Security Configuration and Violation Reporting
uskey512
1
450
Snowflake x dbtで作るセキュアでアジャイルなデータ基盤
tsoshiro
2
460
詳細解説! ArrayListの仕組みと実装
yujisoftware
0
500
Importmapを使ったJavaScriptの 読み込みとブラウザアドオンの影響
swamp09
4
1.3k
Featured
See All Featured
The Success of Rails: Ensuring Growth for the Next 100 Years
eileencodes
43
6.6k
JavaScript: Past, Present, and Future - NDC Porto 2020
reverentgeek
47
5k
Making the Leap to Tech Lead
cromwellryan
133
8.9k
How to Think Like a Performance Engineer
csswizardry
20
1.1k
A Philosophy of Restraint
colly
203
16k
How to Ace a Technical Interview
jacobian
275
23k
The World Runs on Bad Software
bkeepers
PRO
65
11k
Statistics for Hackers
jakevdp
796
220k
How To Stay Up To Date on Web Technology
chriscoyier
788
250k
The Cult of Friendly URLs
andyhume
78
6k
Stop Working from a Prison Cell
hatefulcrawdad
267
20k
Producing Creativity
orderedlist
PRO
341
39k
Transcript
ソースコードを美しくたもつために ~コードレビューの認知限界を突破し、年間400リリースを達成する~ @kotauchisunsun
自己紹介 ~仕事編~ • @kotauchisunsun • 株式会社STYLY 2019年入社 • サーバーサイド責任者 • プラットフォーム部
EM
自己紹介 ~個人編~
今日お話しする話の元ネタ
サーバーサイドのリリース回数が年間418回を達成 最大は1か月80回!!
サーバーサイドのソースコードの自動テストのカバレッジ (Googleの平均テストカバレッジは79%らしい・・・・)
当時の開発体制 委託 委託 企画・設計 開発 レビュー
当時の開発体制 委託 委託 企画・設計 開発 レビュー PRレビューが全部私に来る
年間400件超のコードレビューを頑張った話
コードレビューの効率をあげたい
コードの見る行数を減らそう
コードレビューの着眼点 仕様とプログラムが 合致しているか それ以外 • それ以外 ◦ タイポしている ◦ 1行が長すぎる
◦ 1クラスが大きすぎる ◦ 関数の引数が多すぎる ◦ 非推奨の関数を使っている ◦ コードのにおい ◦ バッドプラクティス 「それ以外」を見る工数を限りなく削減しよう
Linter/静的解析の導入 PHP-CS-Fixer PSR-2(コード規約チェック) PHPMetrics LCOM・循環複雑度 SonarQube コードのにおいの検出 複数の視点からコードレビューの自動化を行った
コードレビューの効率はあがった
なぜか?
コードレビューは難しい
コード 原則1 原則2 原則3 ・・・ 原則N ②自分の脳内の原理原則一覧を列挙し、 ③該当する原則を思い出し、 ④その原則の妥当性を示し、 ①原則に違反してそうな部分を見つけ、
⑥修正方針を示す “仕様と実装の一致以外”ー≒”コードの良さ”のコードレビュー 結構大変。 レビュー ⑤コードが原則に違反している こ とを示し、 例:動詞から始まらない関数の命名
SonarQube(SonarCloud)の場合 ①どのようなコードのにおいか? ②ソースのどの箇所で発生してるか? ③どのように修正すべきか? SonarQubeがレビューのフローを ほぼ自動でやってくれる
コードレビューで考えることが少なくなった
コードレビューの効率はあがった
それでもうまくいかない部分があった
WebApiInterface コードの一貫性の破れ UserController PostController FavoriteHandler MessageController ほかのクラス群はControllerで統一されて いるのにクラスの命名がHandlerとして実 装してしまう。 既存のツールでは対応できない。
PHPのリフレクション機能 プログラム内で定義されたクラスの名前が全て 取得できる関数 各クラスの定義情報が取得できるクラス
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のテストケースとして相乗り。
ルールの一例 • Controllerパッケージのクラス名はControllerをsuffixとする • Applicationパッケージのクラス名はApplicationをsuffixとする • コンストラクタの引数には型必須 • 関数の引数には型必須 •
関数の返り値には型必須 • ファイルの行数は1,000行以下 • 作成日時(created_at)など日時をあらわす引数は日時型(Carbon)とする PHP-CS-Fixerで実現できない細かいルールも追加 社内独自の一貫性も確保できるようになった
別に意外な副産物があった
今までのコードレビュー 既存コード 委託 追加コード このコードは既存コードのルー ルと逸脱しており、 社内では〇〇という ルールで実装しています。 修正お願いします。
Linterを作成しての気づき 委託 既存コード 追加コード ルール1 Linter1 自分では曖昧性のないルールで運用しているつもりだが、 Linterのルール として実装できない矛盾を含んだルールだった。 ルール2
Linter2 Linterとして実装できて、自分としては守っているつもりのルール が、既存コードが守られていなかった。 • いかに自分のコード (一貫性)に関する認識が甘かったかを痛感。 • 論理矛盾のないルールは検証なしで設計できない。 • 厳格で精緻な運用ができるしすべきと思うならプログラムに実装できるはずで、 それなら自分で実装して運用すべき。
もう1つの効果
当初のリリースサイクル 委託 開発 レビュー 開発 レビュー リリース コードの品質を上げる方法が コードレビューしかない。 人力コードレビューがボトルネック
改善後のリリースサイクル 委託 開発 開発 レビュー リリース 既存ツール 自作Linter チェック コードの品質が上がるので
・レビュー時間が短縮 ・差し戻し回数が削減 開発からリリースまでの時間が短縮 →年間400超リリース出来た コードの品質向上が開発者で完結 レビュー待ち時間の削減 開発時間の増加
コードレビューの観点の再分解と効率化 仕様とプログラムが 合致しているか それ以外 既存ツールで対処 自作Linterで対処 一般的なプラクティス 社内固有プラクティス 仕様とプログラムが 合致しているか
ツールにより見るべきソースコードが削減できた。 →大量のレビューができるようになった。
直感的には正しそう。
本当?
別視点から整理してみたい
18章 近年のコードレビュー ジェイソン・コーエン(Jason Cohen)
コードレビュー時間と欠陥の発見数の関係 (引用) ところが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. 横軸:レビュー時間 縦軸:欠陥数
レビュー速度と欠陥密度の関係 (引用) 問題は、400~500LOC/時を超える 早いレビューにあります。 突然、欠陥密度の高いレビューは 稀になります。(中略) レビュー担当者が速く進めすぎて 欠陥を見つけ損なっているからです。 速すぎるコードレビューはバグを見逃すので、 レビュー速度は400行/時未満にすべき
Cohen, Jason. 2006. Best Kept Secrets of Peer Code Review. Baberly, MA: SmartBear Software 横軸:レビュー速度(LOC/時) 縦軸:欠陥密度(欠陥数/kLOC)
コードレビューの限界 (引用) レビューは最大で1時間にすべきであり、(中略) 1回 のレビュー対象は400行を超えてはいけないという結 論になります。 • 1回のコードレビューは1時間まで • 1回のコードレビューは400行まで
人間のコードレビューの量に限界がある。 横軸:レビュー対象の行数(LOC) 縦軸:欠陥密度(欠陥数/レビュー時間)
既存ツールの導入や 自作ツールの開発により コードレビュー量は増やせる。 人間のコードレビューには 限界がある。 1回1時間、400行までしか コードレビューができない。 矛盾?
ちょっと別の話
認知負荷理論 参考:https://atlassian-teambook.jp/_ct/17574839 オーストラリアの教育心理学者、ジョン・スウェラー(John Sweller)氏によって1980年代に開発・確立された理論 脳のワーキングメモリが常に処理している「情報量」を指 し、脳への情報のインプットが私たちの認知能力にどのよう な負担をかけるかを表している。
認知負荷の3つのタイプ • 課題内在性負荷(Intrinsic Cognitive Load) ◦ 情報の内容を理解するのが困難な度合い • 課題外在性負荷(Extraneous Cognitive
Load) ◦ 気を散らせる情報や事象といった外的要因によって引き起こされる認知の 負荷 • 学習関連負荷(Germane Cognitive Load) ◦ 脳がスキーム(世界を理解するための枠組み)を構築しているときに発生す る。スキームの構築・蓄積により、同様な状況で適切な判断ができるように なる。 参考:https://atlassian-teambook.jp/_ct/17574839
ダンスを学ぶ時の認知負荷の解釈 音楽に合わせて、腕を振 り上げて・・・・ 右足をあげて、右手をに ぎって、上体を反らして ・・・ ダンスを学ぶために スマホで検索して・・・ 課題内在性負荷 学習関連負荷
課題外在性負荷 ※発表者は専門家ではありません
コードレビューの認知資源の内訳 課題 内在性 負荷 課題 外在性 負荷 人間の耐えられる認知負荷の限界 課題 外在性
負荷 課題 内在性 負荷 余力 課題外在性負荷を 減らすことができれば、レ ビュー量が増やせる?
コードレビュー効率化の実践と認知負荷理論の接合 仕様とプログラムが 合致しているか (課題内在性負荷) それ以外 (課題外在性負荷) 一般的なプラクティス 社内固有プラクティス 仕様とプログラムが 合致しているか
(課題内在性負荷) 人間の耐えられる認知負荷の限界 = 人間のコードレビューできる限界 コードレビューできる規模が増えた 自作・既存ツールで 節約した情報量 MakingSoftwareの結論と認知負荷理論に矛盾することなく今回の手法と成果を整理できた。 →今回のレビュー効率化の手法はおそらく有効そう?
まとめ • 2022年にコードレビューが急増した • コードレビューの効率化を行った ◦ 既存ツールの導入・自作Linterの開発 • 自作Linterの開発で自分のコードに関する認識を改めた ◦
自分のコード規約の矛盾の認知・一貫性の欠如 • コードレビューの改善によりリリースサイクルが短縮化された • コードレビューは1時間の間に、1回400行未満にしないとバグを見逃す危険性が高まる。 • 認知負荷には3種類ある ◦ 課題内在性負荷・課題外在性負荷・学習関連負荷 • 大量のコードレビューが可能になった理由は、既存ツール・自作 Linterの開発により課題外 在性負荷を軽減することで、仕様と実装の不一致の確認に認知資源を注力できたことによ るものだと整理できた • 課題外在性負荷の軽減により年間400超のリリースが実現できた
というわけで
みなさんCI/CDやってますか?
静的解析・Linter動かしてますか?
自動テスト書いてますか?
千里の道も一歩から
がんばっていきましょう。