私はリファクタリングが大好きだ。スパゲッティコードを解きほぐし、その根底にあるロジックを明確で直感的な形で浮かび上がらせることほど満足感を与えてくれるものはない。

リファクタリングには慎重さが求められることを学んだ。若く向こう見ずだった頃は、レガシーなコードベースに飛び込み、制御された変更など気にせずコードを引き裂いていた。そうすると決まって、数日後あるいは数週間後に、一見無関係に思えたが実は稀なケースにとって極めて重要だった微妙な部分を取り除いたことで、コードを壊してしまったことに気づくのだった。

この記事では、慎重にリファクタリングする方法を紹介する。実際のレガシーなPythonライブラリをリファクタリングする際に私が用いたテクニックを解説する。ミスを最小限に抑えるために使った開発ツールチェーンや、既存の挙動を固定するためのユニットテストの追加プロセスも含めて紹介しよう。

これは、機械学習を使って料理の材料表記（例：「2 cups milk」）を構造化データにパースするライブラリ、ingredient-phrase-taggerを私がどのように蘇らせたかについての3部作の最終回だ。詳しい背景はパート1を読んでほしいが、要約すると、放置されていたライブラリを見つけ、自分のSaaSビジネスを支えるために再び動くようにしたという話である：

• パート1：蘇生 - コードを手当てして、どんなモダンな環境でも動作するようにした話
• パート2：安定化 - コードを修復する間、機能が退行しないようにした話
• パート3：リハビリテーション（この記事） - コードのリファクタリングを始めた話

ここまでのおさらい

前の2つの記事で、私はカスタムDockerイメージを作成してこのライブラリをどこでも使えるようにし、エンドツーエンドテストを追加して高レベルな挙動を保護した。コードベースへの変更があるたびに、Travis CIがすべての依存関係をビルドし、制御された環境でテストを実行した。

ここまでは、コード自体には手を加えていなかった。既存のコードの上に、その挙動を検証するためのツールやスクリプトを追加しただけだった。コードを安全に変更するための仕組みがすべて整ったので、ようやくリファクタリングを始められるようになった。

空白の規約を強制する

開発者は空白のことで頭を悩ませるべきではない。新しいソフトウェアプロジェクトを始めるときは、できるだけ早い段階で空白のフォーマットを自動化するようにしている。

Pythonプロジェクトでは、YAPF（Yet Another Python Formatter）でこれを実現している。このプロジェクトで最初に行ったコード変更は、すべてのファイルを私の好みの標準であるGoogleのPythonスタイルガイドに合わせて再フォーマットすることだった：

yapf \
  --in-place \
  --recursive \
  --style google \
  ./ \
  --exclude="third_party/*" \
  --exclude="build/*"

これにより大量の差分が発生したが、YAPFは成熟したツールであり、エンドツーエンドテストも引き続きパスしていたため、安全な変更だと確信していた。

他の変更がノイズに埋もれてプルリクエストのレビューが困難にならないよう、プルリクエストは空白の変更のみに限定するよう注意した。

今後の変更でも同じスタイル規約が守られるように、ビルドスクリプトに新たなチェックを追加した：

yapf \
  --diff \
  --recursive \
  --style google \
  ./ \
  --exclude="third_party/*" \
  --exclude="build/*"

先ほどのコマンドと同じだが、--in-placeフラグの代わりに--diffフラグを使っている。YAPFが空白の違反を検出すると、それを出力し、失敗を示す終了コードを返すことでビルドスクリプトを失敗させる。

静的解析の導入

pyflakesも、Pythonのツールチェーンに必ず加える便利なコンポーネントだ。静的解析を使って、未初期化の変数や未使用のimportといった不注意なミスを見つけてくれる。

ingredient-phrase-taggerのビルドスクリプトに追加したところ、すぐに未使用のimportを検出してくれた：

$ pyflakes \
    bin/ \
    ingredient_phrase_tagger/
ingredient_phrase_tagger/training/utils.py:3: 'string' imported but unused

いよいよコードを読む

この過程を通じて、私がコードを理解しようとする試みを避けてきたことにお気づきかもしれない。ライブラリの挙動について表面的な理解だけでやり過ごしてきたのだ。

コードを読むための最良の方法は、リファクタリングとテストをしながら進めることだと私は気づいた。著名なソフトウェアの専門家であるマーティン・ファウラーが、このプロセスを最も的確に表現している：

馴染みのないコードを見るとき、私はそれが何をしているのか理解しようとしなければならない。数行を眺めて、心の中で「ああ、この部分はこういうことをしているんだな」と思う。リファクタリングでは、その心の中のメモで終わらせない。実際にコードを変えて自分の理解をよりよく反映させ、そしてコードを再実行してそれがまだ動くかどうかで、その理解をテストするのだ。

-マーティン・ファウラー、Refactoring: Improving the Design of Existing Code

貧弱なコード構成への対処

ライブラリのコードの80%は、わずか2つのファイル、cli.py（コマンドラインインターフェース）とutils.py（ユーティリティ）に集中していた。言い換えれば、作者はコードを「ユーザーインターフェース」と「その他すべて」という2つのバケツに分けただけだった。しかし、その分離すらきれいではなかった。

cli.py内のコードで、コマンドラインからの読み書きに関係するものはほとんどなかった。Cliという単一のクラスで構成され、次のようなメソッドを持っていた：

• run
• generate_data
• parseNumbers
• matchUp
• addPrefixes
• bestTag
• _parse_args

まず取りかかったのは、Cliクラスをスリム化し、コマンドラインインターフェースとしてより論理的な抽象化にすることだった。

Cliクラスの解剖

Cliクラスを分割するために、取っかかりが必要だった。generate_dataはどう見てもユーザーインターフェースを担うクラスに属すべきものには思えなかったが、すぐには移動できなかった。generate_dataはselfパラメータを通じてCliの他のメソッドを呼び出しており、クラスの他の部分と状態を共有しているように見えたからだ。

いや、本当にそうだろうか。cli.py内のすべての関数はCliクラスのメンバー（メソッド）だったが、実際にインスタンス変数を共有していたのだろうか。

Cliのコンストラクタを確認した：

def __init__(self, argv):
      self.opts = self._parse_args(argv)
      self._upstream_cursor = None

コンストラクタはself._upstream_cursorに値を代入していたが、その変数を参照している箇所はどこにもなかった。デッドコードだったので、簡単に削除できた。

もう一つのメンバー変数であるself.optsはデッドコードではなかったが、それを参照していたのはrunとgenerate_dataの2つのメソッドだけだった。

共有される状態がなければ、Cliの他のpublicメソッドがそもそもメソッドである理由はなかった。それらはすべてモジュールレベルの独立した関数として存在できるし、さらに言えば、cliよりもその目的を的確に表すまったく新しいモジュールに移動することさえできた。

クリーンな抽象化を作る

Cliのメソッドのほとんどを別のモジュールに移せることが分かると、その新しいモジュールを設計しなければならなかった。もちろん、すべての関数をそこに移してすべてpublicにすることもできたが、Cliクラスと新しいモジュールの間に最小限のインターフェースを作りたかった。

Cliは他のすべての関数をgenerate_dataのループ本体の中で呼び出していることに気づいた。そのコードを新しい関数として抽出すれば、Cliはその新しい関数だけにアクセスすればよく、以前のメソッドには一切触れる必要がなくなる。

この変更により、Cliクラスはよりスリムで論理的に凝集したものになった。今では2つのpublicメソッドと1つのprivateメソッドだけから構成されるようになった：

• run
• generate_data
• _parse_args

まだ完璧ではなかったが、以前の肥大化したインターフェースよりはましになった。もちろん、他にも加えたい変更はたくさんあったが、それらは後回しにしなければならなかった。

ミスの可能性を最小限に抑えるため、リファクタリングでは各プルリクエストのスコープを絞った。ファイル間でコードを移動する際は、移動自体が行レベルの変更に気づきにくくするため、変更を最小限に抑えることが特に重要だ。

エンドツーエンドテストがパスしたことで、移動によって重大なものを壊していないことは分かったが、作業はまだ終わりではなかった。リファクタリングで新しい関数を作ったのだから、それを検証する新しいユニットテストが必要だった。

初めてのユニットテスト

ユニットテストの作成は簡単だった。translator.translate_rowの開始時と終了時に一時的にデバッグログを仕込み、入力と出力を出力させた。その値が、最初のユニットテストの入力と期待される出力になった：

def test_translates_row_with_simple_phrase(self):
    row = {
        'index': 162,
        'input': '2 cups flour',
        'name': 'flour',
        'qty': 2.0,
        'range_end': 0.0,
        'unit': 'cup',
        'comment': '',
    }
     self.assertMultiLineEqual("""
2\tI1\tL4\tNoCAP\tNoPAREN\tB-QTY
cups\tI2\tL4\tNoCAP\tNoPAREN\tB-UNIT
flour\tI3\tL4\tNoCAP\tNoPAREN\tB-NAME
""".strip(),
                              translator.translate_row(row).strip())

この関数が何をしているのか、まだ完全には理解できていなかったが、ユニットテストによって理解に近づいた。これがライブラリの学習データを処理していることが分かった。そのデータは次のようなCSVファイルに格納されていた：

関数は、ライブラリの機械学習エンジンが理解できるタブ区切りの値のセットを返していた。

さらにいくつかのユニットテストを追加して、異なるタイプの材料をカバーした。分数を含む材料（"1 1/2 teaspoons salt"）や、コメントが付いた材料（"Half a vanilla bean, split lengthwise, seeds scraped"）などである。

ユニットテストをビルドに統合する

ユニットテストはビルドプロセスに統合されなければあまり意味がないので、ビルドスクリプトを更新して含めるようにした：

Travis CIがすでにコード変更のたびにビルドスクリプトを実行していたため、次のTravisビルドでユニットテストの出力を見ることができた：

コードカバレッジの追加

リファクタリング中は、より多くのコードをテストのカバー下に置くにつれて、コードカバレッジの数値が上がっていくのを見るのが大好きだ。Pythonプロジェクトでは、カバレッジ情報を収集するのにcoverageモジュールを、結果をウェブダッシュボードで確認するのにCoverallsを使っている。

Python標準のユニットテストランナーからcoverageへの切り替えは、ビルドスクリプトのわずかな変更で済んだ：

-python -m unittest discover
+coverage run -m unittest discover

続いて、Travisがコードカバレッジ情報をCoverallsにアップロードするように、Travis設定にafter_successキーを追加した。

after_success:
  - pip install pyyaml coveralls
  - coveralls

期待に胸を膨らませてCoverallsを確認すると……

何も表示されなかった。

コードカバレッジはどこへ行った？

過去に何十ものプロジェクトでCoverallsを使ってきたので、なぜ何も表示されないのか理解できなかった。ただのシンプルなPythonプロジェクトだ。coverageコマンドは.coverageというファイルにコードカバレッジ情報を作成し、coverallsコマンドがそれをCoverallsのダッシュボードにアップロードするはずだった。

ああ、そういうことか！coverageコマンドはDockerコンテナ内で実行されていたが、coverallsバイナリは通常のTravis環境で実行されていたため、.coverageファイルを見つけられなかったのだ。Dockerコンテナから外側のTravis環境へファイルをコピーしていなかったのである。

これは簡単に修正できた。Dockerコンテナから.coverageファイルを抽出するコマンドを追加するだけだった：

after_success:
  - pip install pyyaml coveralls
  - docker cp ingredient-phrase-tagger-container:/app/.coverage ./
  - coveralls

それでも、Coverallsのダッシュボードには何も表示されなかった：

しかし、Travisビルドには以前のビルドにはなかった出力が表示されていた：

$ coveralls
Submitting coverage to coveralls.io...
No source for /app/ingredient_phrase_tagger/__init__.py
No source for /app/ingredient_phrase_tagger/training/__init__.py
No source for /app/ingredient_phrase_tagger/training/cli.py
No source for /app/ingredient_phrase_tagger/training/translator.py
No source for /app/ingredient_phrase_tagger/training/utils.py
Coverage submitted!
Job #177.1
https://coveralls.io/jobs/39259674

そこで、別の問題があることに気づいた。

TravisとDockerでは、ファイルシステムの見え方が競合していた。例えば、それぞれがcli.pyファイルをどのように見ていたかは次のとおりだ：

それを踏まえると、Travisでcoverallsが出力したエラーメッセージにも納得がいった：

No source for /app/ingredient_phrase_tagger/training/cli.py

Coverallsがファイルを見つけられなかったのは、.coverage内のパスがDockerコンテナ側のファイルシステムの見え方に基づいていたからだ。/appというパスはTravisのファイルシステムには存在しなかった。

同じファイルに対して互換性のない見え方をする2つの異なる環境のギャップを、どうやって埋めればいいのか。解決策は見つかったが、少々回りくどいものだった。

パスを変換する回りくどい方法

coverageのドキュメントで、複数のファイルシステムからのパスを結合することについて述べたpathsオプションをサポートしていることに気づいた：

これらのオプションを使うために、次のような.coveragercファイルを作成した：

[run]
source = ingredient_phrase_tagger

; Run in parallel mode so that coverage can canonicalize the source paths
; regardless of whether it runs locally or within a Docker container.
parallel = True

[paths]
; the first path is the path on the local filesystem
; the second path is the path as it appears within the Docker container
source =
  ingredient_phrase_tagger/
  /app/ingredient_phrase_tagger/

新しい解決策では、Dockerコンテナ内でcoverageコマンドを実行し、次にTravis環境でcoverage combine機能を実行して、すべてのパスをTravisのファイルシステムに正規化した。

この解決策を適用した後、Travis設定のafter_successセクションは次のようになった：

after_success:
  - pip install pyyaml coveralls
  # Copy the .coverage.* file from the Docker container to the local filesystem.
  - docker cp ingredient-phrase-tagger-container:/app/$(docker exec -it ingredient-phrase-tagger-container bash -c "ls -a .coverage.*" | tr -d '\r') ./
  # Use coverage combine to canonicalize the source paths.
  - coverage combine
  # Upload coverage information to Coveralls.
  - coveralls

ついにコードカバレッジを取得

完全な解決策をテストしてみた。ついにCoverallsが結果を受信し、コードカバレッジの数値を表示してくれた：

このライブラリの復活をここに宣言する

コードカバレッジのトラッキングを統合した後、このライブラリは再び息を吹き返したと感じた。品質で賞を取れるようなものではないが、私や他の開発者が高い信頼性を持ってコードの改善を続けられるだけのインフラは整った。

この一連の記事では、ライブラリを小さく個別のステップで改善していく過程を説明してきた。これによりバグの可能性は最小限に抑えられたが、全体像が見えにくくなったかもしれない。少し視点を変えて、蘇生の過程で行った高レベルな改善を振り返ってみよう：

捨てるつもりでリファクタリングする

これらの変更をこれほど誇らしく思っていたのだから、さらに数週間コードを改善した後に、それを捨てて全面的に書き直すことにした、と聞けば驚かれるかもしれない。

……一つは捨てるつもりで計画せよ。どうせ捨てることになるのだから。

-フレッド・ブルックス、The Mythical Man-Month: Essays on Software Engineering

コードをリファクタリングすればするほど、根本的なアーキテクチャに問題があることが分かってきた。だからといって、コード改善の努力が無駄だったわけではない——深い理解を得るためには手を汚す必要があったのだ。すべてを理解したとき、より良い保守性とパフォーマンスのために一から書き直すことに安心感を覚えた。

その結果生まれたのが、Zestfulというサービスだ。ingredient-phrase-taggerと似た機能を提供するが、ホストされたAPIとして提供される。私がオリジナルのライブラリを機能させるためにくぐり抜けたような面倒な手順を踏むことなく、クライアントはすぐに材料のパースを行える。

Zestfulの動作を見てみたい方は、ライブデモをチェックしてほしい：

カバーイラストはLoraine Yowによるもの。ingredient-phrase-taggerライブラリの私のフォークはGitHubで公開している。このライブラリをベースにしたマネージドサービスZestfulも提供している。