幾何処理するサービスをリファクタする時にやって良かったことリスト
そろそろwebエンジニアを名乗るしかなくなってしまったyasu_umiです。
この記事は、AEC and Related Tech Advent Calendar 2021に合わせて書かれたものであり、実験的に実装されたコード山盛りの状態でそのままプロダクトの一部として実働を初めてしまったサービスをPONと渡された方向けになります。
まずは表題を回収しておきます。
やって良かったことリスト
ここまで読んだ多くの人が、なーんだそんな話か(そっ閉じ…となっていることを切に願います。
まずは状況の確認から
それでは本題に入りましょう。
"実験的に実装されたコード山盛りの状態でそのままプロダクトの一部として実働を初めてしまったサービス"とは一体どのような状態か、深刻度順に行くと…
- テストがない
- linterがない
- CIしてない
- 手元の実行環境が本番と合ってるか怪しい
この辺りを満たすものとします。この状態のサービスにおける機能追加は
- コードを書く
- 何となく手元で動かしてみる
- 以前と結果が違ったり違わなかったりするのを目視で確認する
- リリースしてみる
というサイクルになるでしょう。この状態でサービス開発に人を追加投入するのは非常に困難です。新しく開発に関わる人は、以前と結果が違ったり違わなかったりするのを目視で確認することができません。*2
ということで、新しく開発に関わる人こと僕は、幸いにもこのサービスは開発が活発なわけではなく、しばらくの間であれば開発を完全に止められるという状況もあり、大規模なリファクタに着手することになりました。
リファクタするぞ!の前に(実行環境編)
それではリファクタを…始めてはいけません! 事故る!!
この状態のコードに大幅な変更を加えるのは自殺行為です。そもそも、リファクタが必要なのは結果が変わった時に確認する方法がないからです。この状態での大規模なリファクタは間違いなくバグを埋め込んでしまい、大量のコード変更のどこでバグったのかすらわからなくなるでしょう。
そこでまずは手元の実行環境を整えましょう。
PaaSを使っているにせよ何にせよ、手元で本番環境と同じ環境で動作確認できるようにしましょう。具体的にはdockerを使って言語やランタイムを揃え、パッケージ管理システムを使ってライブラリのバージョンを固定します。
注意点は、ここでパッケージを更新したり言語のバージョンを上げたりしない、ということです。まずは動いているものに全ての状況を揃えましょう。
リファクタするぞ!の前に(E2Eテスト編)
じゃあ準備もできたしリファクタを…始められない! バグる!!
動作環境は整ったものの、結果の動作結果の確認は変わらず目視です。幾何処理するサービスにありがちな問題ですが、結果が複雑すぎてテストを書くのが大変難しいかもしれません。そんな時に有効なのは雑なE2Eテストです。
サービスインしているということは、ユーザからの入力と出力があるはずです。もしかしたら過去にバグった事例があり、その時の修正確認に使った入力が残っていれば最高です。どんな形であれ、まずは今のサービスからの出力を取りましょう。
これを使ってE2Eテストを書くことで、最低限既存と同じような動作をすることを保証しつつ、中を弄ることが可能になります。
注意点は、ここでユニットテストを書かない、ということです。この段階ではコードの中身はまだしっちゃかめっちゃかでこの後大規模にリファクタするため、この時点で頑張ってコードを読み解きユニットテストを書いても、ほぼ無駄になってしまうでしょう。
リファクタするぞ!の前に(linter&CI編)
じゃあE2Eテストで安全にリファクタできるようになったし、今度こそリファクタするぞ!
でも良いのですが、もう1ステップだけ間に挟みましょう。linterです。
コードの書き方は様々です。同じ機能を実装するにしても様々な書き方があり、現状では何の縛りもなくあらゆる書き方のコードが許されてしまいます。これでは個々の書き手の趣味全開のコードがガンガン入ってしまい書き方が安定せず、読む時に大変です。そこでlinterを入れ、書き方に一定の制約を加えられるようにしましょう。
また、linterは手元で実行できるだけでは誰も守りません。github actionsなどでpull reqマージ前に必ずlinterを通し、通っていないコードはmergeできないようにしましょう。その2で追加したE2Eテストも必ず通してからmergeされるようにしましょう。
リファクタするぞ!
ついにリファクタです。まずは設計から、といきたいところですが、中身のわからないコードの設計を見直すのは大変です。オススメは長い関数の分割をしつつコード読み、です。ある程度関数分割が進んだら、次はクラスの見直しです。この段階まで来たら、コードの中身もある程度理解でき、ユニットテストを書き始められるでしょう。
リファクタの先に
"リファクタは盆栽"*3とは良く言ったもので、リファクタに終わりはありません。CIでのチェックなどを入れたとはいえ、破壊することは非常に容易です。特にチームで開発している場合、リファクタ直後のコードの状態を維持していくにはチームの協力が不可欠なので、linterの内容はテストカバレッジの確認などなど、チームとしてどうしていくか相談しましょう。
余談
具体的な話を削ったところ、当たり前かつ幾何処理とほぼ関係ない話になってしまいました。
それでも、幾何処理するサービス*4特有の問題を上げるとすると、
一点目は、ロジックの変更に伴なうE2Eテストの結果変更の是非判断が非常に難しいことです。例えばPolylineのSegment分割ロジックを変更したとすると、結果のPolylineの座標が増減したり変化したりしますが、これをgithub上でreviewすることは困難です。つまりE2Eテストはあくまで命綱であり、開発サイクルの中でこれを頼りにすることはできません。
二点目は、ユニットテストを書くことは容易いということです。処理の分割さえ適切に行えば、そして、サービスが複雑な内部状態を持たなければ*5という2つの但し書きはつきますが。
ということで、これらの作業はコードが少ない方が簡単です。コードを書き始める時にやっておけば、大規模なリファクタなんてやらなくても、コスト0で綺麗なプロジェクトがスタートできますし、コードの内容を覚えているうちならユニットテストを書くのはさらに簡単です。
オマケ
さて、ここからは具体の話です。この話は2つのプロジェクトにおいて起こった問題を纏めたものであり、1つはnodejs, もう1つはpythonのプロジェクトです。
まずは1つ目のnodejs。package.jsonから関係のあるところを抜粋すると
{ "devDependencies": { "@typescript-eslint/eslint-plugin": "^4.33.0", "@typescript-eslint/parser": "^4.30.0", "eslint": "^7.31.0", "eslint-config-prettier": "^8.3.0", "eslint-plugin-import": "^2.23.4", "eslint-plugin-jest": "^24.4.0", "eslint-plugin-prettier": "^3.4.0", "eslint-plugin-promise": "^5.1.0", "jest": "^27.0.6", "pprof": "^3.2.0", "prettier": "^2.3.2", "ts-jest": "^27.0.5", "ts-node": "^10.2.1", "typescript": "^4.4.2" }, "eslintConfig": { "env": { "node": true, "es2020": true }, "extends": [ "eslint:recommended", "plugin:@typescript-eslint/recommended", "plugin:promise/recommended", "plugin:prettier/recommended", "plugin:jest/recommended", "plugin:jest/style", "prettier" ], "parser": "@typescript-eslint/parser", "parserOptions": { "ecmaVersion": 11, "sourceType": "module" }, "plugins": [ "@typescript-eslint", "promise", "import", "jest" ], "rules": { "@typescript-eslint/naming-convention": [ "error" ], "@typescript-eslint/explicit-module-boundary-types": "off", "no-use-before-define": [ 0 ], "import/order": [ "error", { "newlines-between": "always", "alphabetize": { "order": "asc" } } ] } }, "prettier": { "tabWidth": 2, "singleQuote": true, "trailingComma": "all" }, "jest": { "preset": "ts-jest", "testEnvironment": "node", "coveragePathIgnorePatterns": [ "/node_modules/" ] } }
こんな感じ。受け取った時点では生jsで書かれていたので、まずはtypescriptを入れてjestでE2Eのテストを書いて…という流れでした。
そもそもの問題として実行が非常に遅いアプリケーションであるのをどうにかしたい、というのが本題だったので、pprofはそのために使いました。この記事にあるような内容を終えた後には、ts-nodeでframe graphを見るなど。見え方は先達の記事を参考まで。
次にpython。こちらはpipenvを導入しかけた形跡があるものの、実態としてはrequirements.txtを使っており…全部やめてpoetryにしたのでpyproject.tomlから関係のあるところを抜粋すると
[tool.poetry.dev-dependencies] pycln = "^0.0.4" isort = "^5.9.2" autopep8 = "^1.5.7" add-trailing-comma = "^2.1.0" black = "^21.7b0" flake8 = "^4.0.1" flake8-commas = "^2.1.0" flake8-comprehensions = "^3.7.0" flake8-quotes = "^3.3.1" mypy = "^0.910" [tool.isort] profile = "black" line_length = 120 multi_line_output = 3 include_trailing_comma = true overwrite_in_place = true
こんな感じ。pythonのlinter, formatterはnodejsと比較すると群雄割拠状態であり、まだこれが最強、という組み合わせを見つけられずにいます。
これはどちらのサービスにも当て嵌ったことですが、幾何処理系のライブラリは技術が枯れているのかメンテされていないライブラリがメジャーに使われているのを見受けました。 2年以上更新がなかったら諦めてforkしてしまい、自力メンテを視野に入れてよいでしょう。もちろんpull reqを出してmergeしてもらうのが一番ですが。 また、このようにメンテされていなくて良くわからないライブラリを使ってしまっている場合、影響範囲を局所化することは何よりも大事です。 古いライブラリになるほど、nodejsであればtypescriptの型定義が、pythonであればmypyの型定義がないものです。これをあちこちで使ってしまうと別のものに変えることすら一苦労です。 型定義をサクッと書ければそれが一番良いですが、それが難しそうな場合、用途に合わせて入出力の型注釈をちゃんと書いてテストもあるモジュール内に閉じ込めてしまえば、同様に動作するモジュールに置き替えることで、後日依存を外すことが容易でしょう。
最後に
具体的な話と抽象的な話どちらに需要があるカレンダーなのか迷った結果、結局どちらも書いてしまいました。 10を100にするようなサービス開発をされている方には"こんな当たり前のことを今更"という内容であり、0を1にするような実験的なコードを書かれている方からは"こんな面倒なことやってられるか"という内容でしょう。 1を10にするタイミングで*6、チラとでも思い出していただければ幸甚です。
