2024/01/15 youtube 録画
実録レガシーコード改善
twadaさん(和田卓人さん)
- チャット実況してほしい
- スクショOK
講演の背景
- 以前実施した講演の改訂版
- 前回の講演が映像に残っていないので残したい
- 2018年に実際に行った受託開発
- プロダクトオーナー許可済み
- コードは本物を使用
- 技術が2018のものなので若干古い
シナリオ
スマートスピーカーの開発(都道府県クイズ)
当時スマートスピーカーの開発は初めて
74行のJavaScript
ロジックはAWSLambda
プロのプログラマーでない人のコード
今後も拡張したい
コードコメントも実際にあったもの
主要なロジックの理解
テストコードがない
レガシーコード改善ガイド
- テストのないコードは悪いコード
バージョン管理もされていない
自動化もされていない
- →ソフトウェア開発の3本柱がい一つもない
闘う準備を整える
ソフトウェア開発の3本柱の優先順位
- バージョンコントロール→テスト→自動化
バージョン管理は優先度大
- 秘匿情報まわりだけ注意
自動化はレバレッジが効きやすいので早期着手する
- チーム開発の場合、だれかが作れば全員が受益者になれる
- きれいじゃなくてもよいので手作業を減らす
自動テスト
- E2Eテストがつらい
- 遅いし不安定
- 前線基地
- 今後の機能開発のため
- まずは一本自動テストを通す
- 1件あるだけでも違う
- 脳天気な正常系でもよい
- リファクタリングへの耐性を高める
- 実装の詳細からある程度距離をとりつつ自動テストを安定してできるように
- E2Eテストがつらい
スマートスピーカーとハンドラーの間から
- リクエストレスポンスを狙う
自動テストに使えるライブラリ・フレームワークの調査
- 高機能抽象化・かゆいところに手が届かない
- 機能が少なく、レイヤが薄い 手が届く
- → 情報の少ない戦場には小さく、直交性の高いシンプルな道具を使うので後者
テストを書いてみる
- レスポンスがundefinedでないことのテスト
- リクエストイベントの自作
- →ちゃんと動いた
- レスポンス完全一致のテストに書き換え
- テスト失敗した
- →出力と期待値が一致しない
- →クイズのランダム性が原因
- レガシーコードのジレンマ
- コードを変更するためにテストを整備が必要。テストを整備するにはコードを変更する必要がある。
- 今回のケース
- 接合部を作る
- ランダム性を伴う関数を外から差し込めるようにする
- あぶないコード変更。なるべくペアプロなどする
- テストからは固定値を返すようにしてテスト
- 接合部を作る
- HumbleObjectPattern
- テスト容易性を下げている要素を薄く切りだし、テスト可能範囲を広くとる基本パターン
- レスポンスがundefinedでないことのテスト
機能追加とその結果
仕様変更 *間違えても同じ問題を3回まで繰り返したい
TDDのサイクル
- 次の目標を考えてリストアップする
- リストからひとつピックアップして、その目標を示すテストをひとつ書く
- そのテストを実行して失敗させる(Red)
- 目的のコードを書く
- 2で書いたテストを成功させる(Green)
- テストが通るまでリファクタリングを行う(Refactor)
- 1~6を繰り返す
状況にぶらさがるテストを書くことが多い
使用を満たすコードてテストがオールグリーンまで書いた
- →設計はよくなっているか?
- リファクタリングしないと内部の質が上がらない
引き継いだときよりコードの質が下がってきた
- テストで品質は上がらない。テストはあくまで品質をあげるきっかけ。品質を上げるのはプログラミング
仕様変更の追加
基盤ライブラリのサポートが切れる
- →こんなはずではなかった
後でクリーンにするという言葉で、あとでクリーンにすることはない。市場からのプレッシャーは止まらないから。崩壊が始まる。生産性が0に近づいていく
モデルを分離する
現状のつらさの分析
- 2つの変更要因が単一のプロダクトコードに降り掛かっている
- イベントハンドラにすべてのドメインロジックが書かれている
- ドメインモデルをAlexaとLambdaから切り離す
作戦 Extract 既存のコードにテストを書いて保護しながら抽出←こっちですすめる Sprout 新しく書くこーどだけはテストを書く
ドメインモデルを抽出する
ドメインモデルをテスト駆動開発で新規開発する
PlainOldなモデルクラス
- Lambdaがモデルを利用するようにテスト駆動
詳細への依存を減らす
- モデルの持っている情報をリクエストをまたいでダンプ・リストアできるようにする
- Alexa固有の機能への依存を減らす
- PureJavaScriptで記述
状態遷移もモデル自身が判断できるようにする
- モデルと基盤の結合度を減らせる
結合度を更に減らす
- クラスを公開せず、ファクトリー関数だけ公開することで内部構造を隠蔽
ここまで1日2日で気合で実施
- サイズが大きくない
- テスト駆動になれている
事実と情報を分ける
仕様追加
情報とデータの違い
- 意味のある目的をもった正しい情報
- データは単なる各種の事実の値でそれ自体に目的はない
- 目的をもった情報は、無目的な事実を集積したデータを種々加工して得られる
dumpに混じって入っている
- →事実をモデリングしていく
- 事実・イベントを格納し、情報はそこから計算する
アーキテクチャを定める
要望追加
- 別のスマートスピーカーでも検証行いたい
安定依存の原則:変化しやすいものに依存しない
各スマートスピーカーとコアドメインを依存しないように
アダプターで接続するように
おわりに
TDDは設計のひらめきが正しい瞬間に訪れることを保証するものではない。 しかし、自身を与えてくれるテストときちんと手入れされたコードは、ひらめきへの備えであり、いざひらめいたときに、それを具現化するための備えでもある
- 翻訳して印象に残っている言葉
Q&A
Q. 今回はコード量が少なかったが、多くなっても同じ戦略で行けるか?
A. 場合によるが、数千行の神関数はレガシーコード改善ガイドを参照 サイズ感は違うが、基本戦略は変わらない 大きい変更を小さい変更を積み重ねる形になる違いはある
Q. 大規模なときのチーム体制
A. 場合によるが、メイン部隊とリファクタリングに分けると追いつけなくなりがち 大きい変更を小さい変更を積み重ねる形で
Q. インテグレーションレベルから、ユニットテストに落とす目的やメリット
A. Web DB pressの連載参照。テストピラミッドは下の方が決定性が高く速度が早い。 テスト自動化は短い時間で結果に到達できるかが大事。 数の比率がずれていると不安定になる
感想
- テスト駆動開発の専門家でもレガシーコードの罠に陥ることがあるのだと思った(いわんや一般人をば)
- リファクタリングが必要だと思ったら、そのときにやりきってしまうのが一番なんだろうなと