[PHPカンファレンス沖縄2024]「無理なくできるだけ安全に」テストもないレガシーコードをリファクタリングするテクニック

PHPCON沖縄2024 で登壇した際の資料です。
セッション詳細: https://fortee.jp/phpcon-okinawa-2024/proposal/4584f78e-6281-450d-8e8d-41495a89a6b7

#phpcon_okinawa #track_b

Transcript

1. X(旧twitter): @for__3 #phpcon_okinawa #track_b テストもない レガシーコードを リファクタリングする テクニック PHPカンファレンス沖縄2024@zoe 1 無理なく 安全に
2. X(旧twitter): @for__3 #phpcon_okinawa #track_b 機能改修したいけど、既存機能がぐちゃぐちゃで⼿を加えづ らい‧‧‧ こんな経験ありませんか？ 2
3. X(旧twitter): @for__3 #phpcon_okinawa #track_b 機能改修したいけど、既存機能がぐちゃぐちゃで⼿を加えづ らい‧‧‧ 頑張ってリファクタしたけど、そのせいで不具合が出てし まった‧‧‧ こんな経験ありませんか？ 3
4. X(旧twitter): @for__3 #phpcon_okinawa #track_b こんな経験ありませんか？ 機能改修したいけど、既存機能がぐちゃぐちゃで⼿を加えづ らい‧‧‧ 頑張ってリファクタしたけど、そのせいで不具合が出てし まった‧‧‧ めっちゃ気合⼊れてリファクタしたが、その後改修されるこ となくクローズ‧‧‧ 4
5. X(旧twitter): @for__3 #phpcon_okinawa #track_b リファクタしたいけど、つらい 5
6. X(旧twitter): @for__3 #phpcon_okinawa #track_b リファクタしたいけど、つらい • リファクタしたいところ＝複雑で分かりづらい ◦ 機能追加、改修した際にどこに影響があるかわからない ◦ 結果として→ バグりやすい＝⼿を出しづらい 6 リファクタしたい 複雑でバグりやすい
7. X(旧twitter): @for__3 #phpcon_okinawa #track_b リファクタしたいけど、つらい • リファクタしたいところ＝複雑で分かりづらい ◦ 機能追加、改修した際にどこに影響があるかわからない ◦ 結果として→ バグりやすい＝⼿を出しづらい 7 リファクタしたい ただリファクタすると‧‧‧ 複雑でバグりやすい 障害‧不具合発⽣！
8. X(旧twitter): @for__3 #phpcon_okinawa #track_b リファクタしたいけど、つらい • リファクタしたいところ＝複雑で分かりづらい ◦ 機能追加、改修した際にどこに影響があるかわからない ◦ 結果として→ バグりやすい＝⼿を出しづらい 8 リファクタしたい 複雑でバグりやすい 障害‧不具合発⽣！ ただリファクタすると‧‧‧ リファクタするための テクニックを使おう！
9. X(旧twitter): @for__3 #phpcon_okinawa #track_b • リファクタをするかしないかの判断 • ⼿強いレガシーコードをリファクタするテクニック ◦ リファクタ対象をよく理解する ◦ 利⽤箇所が多い関数をリファクタする ◦ でかいコードは細かく分割していく もくじ 9
10. X(旧twitter): @for__3 #phpcon_okinawa #track_b どういう状況ならリファクタしたほうが良いのか？ 10
11. X(旧twitter): @for__3 #phpcon_okinawa #track_b どういう状況ならリファクタしたほうが良いのか？ • 今後も継続的に改修することがわかっている • すでに不具合などが定期的に発⽣している 11
12. X(旧twitter): @for__3 #phpcon_okinawa #track_b どういう状況ならリファクタしたほうが良いのか？ • 今後も継続的に改修することがわかっている • すでに不具合などが定期的に発⽣している 12 技術負債返済 しないときに かかるコスト 技術負債返済にか かるコスト
13. X(旧twitter): @for__3 #phpcon_okinawa #track_b 技術負債返済にかかるコスト 13
14. X(旧twitter): @for__3 #phpcon_okinawa #track_b 技術負債返済にかかるコスト 単純に返済にかかる⼯数を⾒積もればよいが、経験がないと どれくらい返済に⼯数が掛かりそうか⾒極めづらい 14
15. X(旧twitter): @for__3 #phpcon_okinawa #track_b 技術負債返済にかかるコスト 単純に返済にかかる⼯数を⾒積もればよいが、経験がないと どれくらい返済に⼯数が掛かりそうか⾒極めづらい 本セッションで話す調査のテクニックを利⽤してまずは調査 だけを短時間で⾏うと良い 15
16. X(旧twitter): @for__3 #phpcon_okinawa #track_b 技術負債返済しないときにかかるコスト 技術負債がある部分の今後の開発における影響を考える 16
17. X(旧twitter): @for__3 #phpcon_okinawa #track_b 技術負債返済しないときにかかるコスト 技術負債がある部分の今後の開発における影響を考える • 開発⽣産性に対する悪影響（機能開発の遅れ） 17
18. X(旧twitter): @for__3 #phpcon_okinawa #track_b 技術負債返済しないときにかかるコスト 技術負債がある部分の今後の開発における影響を考える • 開発⽣産性に対する悪影響（機能開発の遅れ） • 機能改修時の不具合の発⽣率上昇 これらを⾒積もるとよい 18
19. X(旧twitter): @for__3 #phpcon_okinawa #track_b どういう状況ならリファクタしたほうが良いのか？ 逆にいうとコストが合わない場合は返済しないという判断 も⼤事である 技術負債返済に かかるコスト 技術負債返済 しないときに かかるコスト
20. X(旧twitter): @for__3 #phpcon_okinawa #track_b 余談:リファクタをする⽬的を明確にしておくのも⼤事 • なんのためにリファクタするのか • どういう⽅針でリファクタするのか • どうなったらリファクタできたと⾔えるのか • ‧‧‧etc 20
21. X(旧twitter): @for__3 #phpcon_okinawa #track_b • リファクタをするかしないかの判断 • ⼿強いレガシーコードをリファクタするテクニック ◦ リファクタ対象をよく理解する ◦ 利⽤箇所が多い関数をリファクタする ◦ でかいコードは細かく分割していく もくじ 21
22. X(旧twitter): @for__3 #phpcon_okinawa #track_b 注意 このセッションではリファクタのスコープとしてコードの 変更で済む範囲に絞って話します ただし、リファクタの際に考えるべきポイントや⼿法⾃体 はアーキテクチャレベルにおいても同様の考え⽅を実践で きるので、参考にしてみてください 22
23. X(旧twitter): @for__3 #phpcon_okinawa #track_b 補⾜: よりきれいにコードを分割していく • コードをよりリーダブルで保守性の⾼いものにしていくた めには、凝集度を上げ結合度を下げていく必要がある 23 凝集度 1. 機能的凝集 2. 逐次的凝集 3. 通信的凝集 4. ⼿順的凝集 5. ⼀時的凝集 6. 論理的凝集 7. 偶発的凝集 結合度 1. 内容結合 2. 共通結合 3. ハイブリッド結合 4. 制御結合 5. スタンプ結合 6. データ結合 本セッションで紹介するテクニックは基本的に結合度を下げるためのテクニック 分かりやすく 機能追加し易い 影響受けにくく 壊れにくい
24. X(旧twitter): @for__3 #phpcon_okinawa #track_b ⼿強いレガシーコードをリファクタするテクニック 24 • リファクタ対象をよく理解する • 利⽤箇所が多い関数をリファクタする • でかいコードは細かく分割していく
25. X(旧twitter): @for__3 #phpcon_okinawa #track_b リファクタ対象をよく理解する 25
26. X(旧twitter): @for__3 #phpcon_okinawa #track_b リファクタ対象をよく理解する • レガシーコードはソースコードから理解できない事が多い 26
27. X(旧twitter): @for__3 #phpcon_okinawa #track_b リファクタ対象をよく理解する • レガシーコードはソースコードから理解できない事が多い • ログを仕込みながら、わかったことはコードコメントに書 いていく ◦ 特に利⽤箇所と利⽤⽅法を明確にしておき、テストが書けるも のは書いておく 27
28. X(旧twitter): @for__3 #phpcon_okinawa #track_b リファクタ対象をよく理解する • レガシーコードはソースコードから理解できない事が多い • ログを仕込みながら、わかったことはコードコメントに書 いていく ◦ 特に利⽤箇所と利⽤⽅法を明確にしておき、テストが書けるも のは書いておく • 過去のPRやコミットメッセージなども活⽤するとよい 28
29. X(旧twitter): @for__3 #phpcon_okinawa #track_b debug_backtrace() を使って利⽤箇所を特定する 29 $limitを指定することで呼び出し元だけを 出⼒したりできる
30. X(旧twitter): @for__3 #phpcon_okinawa #track_b debug_backtrace() を使って利⽤箇所を特定する 30 $limitを指定することで呼び出し元だけを 出⼒したりできる ここでパターンを特定しきれれば、これを元にテストコードに起こすことができるぞ！
31. X(旧twitter): @for__3 #phpcon_okinawa #track_b 31 function traceLog(){ $traces = debug_backtrace(1); foreach($traces as $i => $trace){ if (!empty($trace['object']) && is_object($trace['object'])) { $trace['object'] = get_class($trace['object']).':class'; } if (is_array($trace['args'])) { foreach ($trace['args'] AS &$arg) { if (is_object($arg)) { $arg = get_class($arg).'::class'; } } } $text[$i] = ."#".$i." ".$trace['file'].'('.$trace['line'].') '; $text[$i].= (!empty($trace['object'])?$trace['object'].$trace['type']:''); $text[$i].= $trace['function'].'('.implode(', ',$trace['args']).')'; } echo implode("\n", $text) . "\n"; } debug_backtrace() サンプルコード
32. X(旧twitter): @for__3 #phpcon_okinawa #track_b 32 public function case2($collection) { + traceLog(); $collection = $collection ->filter(function ($i) { return $i % 2 === 0; }) ->values() ->map(function ($i) { return $i * 3; }) debug_backtrace() サンプルコード
33. X(旧twitter): @for__3 #phpcon_okinawa #track_b #0 /var/www/app/Console/Commands/MeasurePerformanceTrait.php(59) App\Console\Commands\ExampleLazyCollection:class->traceLog() #1 /var/www/app/Console/Commands/ExampleLazyCollection.php(36) App\Console\Commands\ExampleLazyCollection:class->case2(Illuminate\Support\LazyColl ection::class) #2 /var/www/vendor/laravel/framework/src/Illuminate/Container/BoundMethod.php(36) App\Console\Commands\ExampleLazyCollection:class->handle() #3 /var/www/vendor/laravel/framework/src/Illuminate/Container/Util.php(41) Illuminate\Container\{closure}() #4 /var/www/vendor/laravel/framework/src/Illuminate/Container/BoundMethod.php(93) unwrapIfClosure(Closure::class) #5 /var/www/vendor/laravel/framework/src/Illuminate/Container/BoundMethod.php(35) callBoundMethod(Illuminate\Foundation\Application::class, Array, Closure::class) 33 debug_backtrace() サンプルコード
34. X(旧twitter): @for__3 #phpcon_okinawa #track_b • リファクタ対象をよく理解する ◦ debug_backtrace()を使う • 利⽤箇所が多い関数をリファクタする • でかいコードは細かく分割していく ⼿強いレガシーコードをリファクタするテクニック 34
35. X(旧twitter): @for__3 #phpcon_okinawa #track_b phpstan/phpstan-deprecation-rules を利⽤する 35
36. X(旧twitter): @for__3 #phpcon_okinawa #track_b phpstan/phpstan-deprecation-rules を利⽤する • リファクタ対象に @deprecated をつけ phpstan-deprecation-rules を使うことで利⽤している箇所 を⼀覧化することもできる 36
37. X(旧twitter): @for__3 #phpcon_okinawa #track_b phpstan/phpstan-deprecation-rules を利⽤する • リファクタ対象に @deprecated をつけ phpstan-deprecation-rules を使うことで利⽤している箇所 を⼀覧化することもできる • ただし、こちらの場合はあくまで静的解析によるもので、 どういう引数が使われてるかまでは出せない 37
38. X(旧twitter): @for__3 #phpcon_okinawa #track_b phpstan/phpstan-deprecation-rules を利⽤する • リファクタ対象に @deprecated をつけ phpstan-deprecation-rules を使うことで利⽤している箇所 を⼀覧化することもできる • ただし、こちらの場合はあくまで静的解析によるもので、 どういう引数が使われてるかまでは出せない • CIと組み合わせてdeprecatedが増えてないことをチェックするのも良い 38
39. X(旧twitter): @for__3 #phpcon_okinawa #track_b 利⽤箇所の特定をなぜやるか リファクタにおける失敗パターンで多いパターンは • 利⽤箇所特定漏れ • 引数だったりの利⽤⽅法の特定漏れ によるものが多いため 39
40. X(旧twitter): @for__3 #phpcon_okinawa #track_b • リファクタ対象をよく理解する ◦ debug_backtrace()を使う ◦ phpstan/phpstan-deprecation-rules を利⽤する • 利⽤箇所が多い関数をリファクタする • でかいコードは細かく分割していく ⼿強いレガシーコードをリファクタするテクニック 40
41. X(旧twitter): @for__3 #phpcon_okinawa #track_b 利⽤箇所が多い関数をリファクタするのは⼤変 41
42. X(旧twitter): @for__3 #phpcon_okinawa #track_b 利⽤箇所が多い関数をリファクタするのは⼤変 • 利⽤箇所が多い関数をリファクタする際は、既存関数と別 で関数を分けてリファクタしよう 42
43. X(旧twitter): @for__3 #phpcon_okinawa #track_b 43 $ find . -name "*.php" | xargs grep 'mbTrim' ./src/app/Http/Controllers/Front/ArticlesController.php: $name = \Text::mbTrim($request->input('name')); ./src/app/Http/Controllers/Front/ArticlesController.php: $title = \Text::mbTrim($request->input('title')); ./src/app/Http/Controllers/Front/ArticlesController.php: $body = \Text::mbTrim($request->input('body')); …etc 例) mbTrimをリファクタしたい
44. X(旧twitter): @for__3 #phpcon_okinawa #track_b 44 class Text { public static function mbTrim($text) { // 元の処理 } + public static function mbTrim2($text) + { + // 新しい処理 + } 例) 新しい処理をmbTrim2として追加
45. X(旧twitter): @for__3 #phpcon_okinawa #track_b 利⽤箇所が多い関数をリファクタするのは⼤変 • 利⽤箇所が多い関数をリファクタする際は、既存関数と別 で関数を分けてリファクタしよう • 既存関数と新規関数をどちらも実⾏して、実⾏結果が違う ときだけログなどに出⼒するようにし、実際のロジックに は既存関数の処理を使うと良い 45
46. X(旧twitter): @for__3 #phpcon_okinawa #track_b 46 //利用箇所 $name = \Text::mbTrim($request->input('name')); + $name2 = \Text::mbTrim2($request->input('name')); + // $name と $name2の結果が違うときだけログを出力 + if ($name !== $name2) { + \Log::info('name: ' . $name . ' name2: ' . $name2); + } 例) 追加したmbTrim2を処理にいれる
47. X(旧twitter): @for__3 #phpcon_okinawa #track_b 利⽤箇所が多い関数をリファクタするのは⼤変 • 利⽤箇所が多い関数をリファクタする際は、既存関数と別 で関数を分けてリファクタしよう • 既存関数と新規関数をどちらも実⾏して、実⾏結果が違う ときだけログなどに出⼒するようにし、実際のロジックに は既存関数の処理を使うと良い 47 → 要は既存ロジックの実⾏結果を元にスナップショットテストを やっているようなイメージ
48. X(旧twitter): @for__3 #phpcon_okinawa #track_b 利⽤箇所が多い関数を「できるだけ安全に」反映 • フィーチャーフラグのように⼀部（社内など）のユーザに だけリファクタした⽅を呼び出すようにしてもよい 48
49. X(旧twitter): @for__3 #phpcon_okinawa #track_b 49 //利用箇所 $name = \Text::mbTrim($request->input('name')); + // featureフラグのときはリファクタしたmbTrim2を使う + if ($onRefactor) { + $name = \Text::mbTrim2($request->input('name')); + } 例) 特定条件のときだけリファクタした関数を使う
50. X(旧twitter): @for__3 #phpcon_okinawa #track_b 50 //利用箇所 $name = \Text::mbTrim($request->input('name')); + // featureフラグのときはリファクタしたmbTrim2を使う + if ($onRefactor) { + $name = \Text::mbTrim2($request->input('name')); + } or + // 1%の確率でリファクタしたmbTrim2を使う + if (mt_rand(1, 100) <= 1) { + $name = \Text::mbTrim2($request->input('name')); + } 例) 特定条件のときだけリファクタした関数を使う
51. X(旧twitter): @for__3 #phpcon_okinawa #track_b 利⽤箇所が多い関数をできるだけ安全に反映 • フィーチャーフラグのように⼀部（社内など）のユーザに だけリファクタした⽅を呼び出すようにしてもよい • 最終的にリファクタした関数に全部移⾏していく⽅針であ れば、既存関数に @deprecated をつけておくのもよい 51
52. X(旧twitter): @for__3 #phpcon_okinawa #track_b • リファクタ対象をよく理解する ◦ debug_backtrace()を使う ◦ phpstan/phpstan-deprecation-rules を利⽤する • 利⽤箇所が多い関数のリファクタテクニック • でかいコードは細かく分割していく ⼿強いレガシーコードをリファクタするテクニック 52
53. X(旧twitter): @for__3 #phpcon_okinawa #track_b でかいコードは細かく分割していく 53
54. X(旧twitter): @for__3 #phpcon_okinawa #track_b でかいコードは細かく分割していく • ⾏数が多く処理が⼤きい関数は、処理の塊単位などで分け て⾏くとよい ◦ ただし、使⽤変数やスコープが変わるので注意 54
55. X(旧twitter): @for__3 #phpcon_okinawa #track_b でかいコードは細かく分割していく • ⾏数が多く処理が⼤きい関数は、処理の塊単位などで分け て⾏くとよい ◦ ただし、使⽤変数やスコープが変わるので注意 • データの取得‧整形‧保存を⼀つの関数でやらない 55
56. X(旧twitter): @for__3 #phpcon_okinawa #track_b でかいコードは細かく分割していく • ⾏数が多く処理が⼤きい関数は、処理の塊単位などで分け て⾏くとよい ◦ ただし、使⽤変数やスコープが変わるので注意 • データの取得‧整形‧保存を⼀つの関数でやらない • ここでも分割前後で関数を実⾏するように動作確認をする 56
57. X(旧twitter): @for__3 #phpcon_okinawa #track_b 補⾜: よりきれいにコードを分割していく • コードをよりリーダブルで保守性の⾼いものにしていくた めには、凝集度を上げ結合度を下げていく必要がある 57 本セッションで紹介するテクニックは基本的に結合度を下げるためのテクニック 分かりやすく 機能追加し易い 影響受けにくく 壊れにくい 凝集度 1. 機能的凝集 2. 逐次的凝集 3. 通信的凝集 4. ⼿順的凝集 5. ⼀時的凝集 6. 論理的凝集 7. 偶発的凝集 結合度 1. 内容結合 2. 共通結合 3. ハイブリッド結合 4. 制御結合 5. スタンプ結合 6. データ結合
58. X(旧twitter): @for__3 #phpcon_okinawa #track_b テストコードを書く ここまでやるとでかいロジックは細かい関数に分割され、 ロジックは疎結合になり、テストコードも書きやすい状態 になっているはず debug_backtrace()などを使って得たケースを元に多い ケースからテストコードにしていこう！ 58
59. X(旧twitter): @for__3 #phpcon_okinawa #track_b 59 株式会社ウィルゲート 10年⽬ シニアマネージャー∕VPoE やってること - 教育∕1on1∕採⽤ - PM∕SRE∕インフラ 興味あること - オブザーバビリティ∕⾃動化∕開発⽣産性∕PHP - 沖縄の美味しいご飯 池添 誠(いけぞえ まこと)
60. X(旧twitter): @for__3 #phpcon_okinawa #track_b 「無理なく安全に」 リファクタリング していきましょう！！ 60 懇親会で話しましょう！