読者です 読者をやめる 読者になる 読者になる

ミニマムリリースを意識していたらコードが肥大化していた話

Misoca開発チームの黒曜です。

仙台へ温泉旅行に行ったついでに、アースキャンディが一時期話題になっていた仙台市天文台に足を伸ばしてみました。
常設展示やプラネタリウムも良かったのですが、なにより口径1.3mのひとみ反射望遠鏡が大迫力で素晴らしかったです。
夜は温泉宿へ行ったので観望会には参加できなかったのが残念でした。
Misocaならリモートワークができるので、いっそ仙台に長期滞在してみようか…

f:id:kokuyouwind:20160317181732j:plain

受発注機能とミニマムリリース

さて、Misocaでは最近、受発注に関する機能を強化しています
このブログを書いている時点で見積書をメールで送信すると、サイト上で見積に関するやりとりや発注を行えるようになっています。

f:id:kokuyouwind:20160318133300p:plain

しかし、受発注に関する一番最初のリリースは「通常の見積書受信画面に発注ボタンが出て、発注通知を送れる」というだけの大変シンプルなものでした。

f:id:kokuyouwind:20160318133458p:plain

受発注に関する機能はなるべくミニマムリリースを意識して進めていたため、当初は既存の見積書受信画面に相乗りする形で実装し、徐々に機能を追加していきました。
これにより、「実装されたものが実際に使われるか」「どんな要望が出てくるか」を確認しながら進めることができたので、よい進め方だったと思います。

起こってしまった問題

しかしながら、実装上はちょっとした問題がありました。

もともと割と複雑だった既存機能のControllerに、さらに新規の機能を次々と追加していったことで、Controllerが非常に複雑になってしまったのです。
小さく分けた機能を実装することに注目したため、あまり将来に繋がらない一時しのぎの処理が増えてしまったことも複雑さに拍車をかけていました。

当然、Controllerが複雑になったぶんだけControllerSpecも複雑になりました。
その行数、実に600行。
元から300行ある複雑なSpecでしたが、更に倍に膨れ上がってしまいました。
しかもその内部に既存機能と新規機能それぞれのセットアップやテストが入り混じっている状態です。
当然ながらこの状態になると保守がしんどくなり、「機能を足したり直したりするのはいいけど、Specに反映したくない…」といった雰囲気が生まれてきてしまいました。

ControllerSpecのリファクタリング

このままではまずいということで、思い切ってControllerSpecの大規模なリファクタリングを行いました。
内容はshared_contextやshared_examplesの切り出し、リソースに合わせたファイル分割など定番の措置ですが、なにぶん600行あるので予想外の位置の変数を見ていたり、shared_examplesの切り出し方に工夫が必要だったり、となかなかの大仕事。

その結果、

f:id:kokuyouwind:20160317183025p:plain

1つのSpecのリファクタリングでfile changes 11, +415行−528行という謎のプルリクエストが生まれることになりました。
元となるファイルが600行程度なのに対して-528行なので、別のファイルに移動しただけという部分がほとんどとはいえ、ほぼ全ての行に変更が入ったことになります。

これにより1ファイルの行数がせいぜい200行程度まで減ったことと、ファイル切り出しの過程でセットアップ処理などの整理が進んだことで、だいぶ保守しやすい状態を取り戻すことができました。
また各テストの見通しが良くなったことにより、コーナーケースの見落としなどが見つかりました。

なおController本体については既存機能との共存のために仕様が複雑になってしまっているため、単純なリファクタリングでは限界がありました。
このため既存機能に囚われず追加機能単体であるべき仕様を検討し、複雑にしている要素自体を削減する措置を行っています。
本来であればこちらを進めることでSpec自体が単純になったはずですが、「仕様変更をSpecに反映するのがつらい」というデッドロック状態だったため、今回はSpecのリファクタリングを優先して進めました。

まとめ

今回の教訓を一言で言い表すと、「塵も積もれば山となる」でしょうか。
小さな機能追加では実装が楽な場所に追加してしまいがちですが、ある程度の規模に育ったタイミングで適切な場所に移すよう心がけていきたいと思います。

またそもそも、プロダクトコードやテストが複雑になる状況では仕様自体が複雑になっている可能性があります。
複雑な仕様はコーディングを難しくするだけでなくユーザーに取ってもわかりづらくなりがちなため、機能の追加だけではなく、不要な機能の削除や仕様の単純化なども重視していきたいと思います。