Pythonでのテストによるリファクタリング：実践例
著者：Leonardo Giordani - 21/07/2017
はじめに
この記事では、テストを活用したリファクタリングの例をご紹介します。テストされていないコードやレガシーコードを扱うとき、リファクタリングは危険です。テストは、正しい方法でリファクタリングを行い、バグの発生を最小限に抑え、場合によっては完全に回避するのに役立ちます。
リファクタリングは簡単ではありません。他の人が書いたコード、あるいは自分が過去に書いたコードを理解し、その一部を移動させたり、単純化したり、一言で言えば改善したりすることは、決して気の弱い人のためのものではありません。プログラミングと同様、リファクタリングにもルールやベストプラクティスがありますが、技術、勘、経験、リスクが混在しているとも言えます。
プログラミングは、結局のところ、職人技なのです。
出発点
この記事では、私たちがアクセスできるサービスAPIで、JSON形式のデータ、つまり以下のような要素のリストを生成するというシンプルなユースケースを紹介します。
code: JSON
{
"age": 20,
"surname": "Frazier",
"name": "John",
"salary": "£28943"
}
これをPythonのデータ構造に変換すると、ageが整数で、残りのフィールドが文字列である辞書のリストが得られます。
誰かが、入力データの統計を計算するクラスを書きました。このクラスはDataStatsと呼ばれ、1つのメソッドstats()を提供しています。入力は、サービスから返されたデータ（JSON形式）と、iageとisalaryという2つの整数です。このクラスの簡単な説明によると、これらは、データセット全体の年ごとの平均昇給額を計算するための初期年齢(iage)と初期給与(isalary)です。
コードは以下のとおりです。
code: python
import math
import json
class DataStats:
def stats(self, data, iage, isalary):
# iage and isalary are the starting age and salary used to
# compute the average yearly increase of salary.
# Compute average yearly increase
average_age_increase = math.floor(
sum([e'age' for e in data])/len(data)) - iage average_salary_increase = math.floor(
sum([int(e'salary'1:) for e in data])/len(data)) - isalary yearly_avg_increase = math.floor(
average_salary_increase/average_age_increase)
# Compute max salary
threshold = '£' + str(max(salaries))
max_salary = [e for e in data if e'salary' == threshold] # Compute min salary
min_salary = [e for e in data if e'salary' == '£{}'.format(str(min(salaries)))]
return json.dumps({
'avg_age': math.floor(sum([e'age' for e in data])/len(data)), 'avg_salary': math.floor(sum(
'avg_yearly_increase': yearly_avg_increase,
'max_salary': max_salary,
'min_salary': min_salary
})
ゴール
前のクラスの問題点を見つけるのは、素人目にもかなり簡単です。その中でも特に目を引くものを挙げてみましょう。
このクラスは単一のメソッドを公開しており、__init__()がないため、同じ機能を単一の関数で提供することができる。
stats()メソッドは大きすぎて、多くのタスクを実行しています。そのため、デバッグが非常に難しくなっています。なぜならば、すべてを実行するコードが1つだからです。
コードの重複が多く、少なくともよく似た行がいくつもあります。特に、'£' + str(max(salaries)) と '£{}'.format(str(min(salaries))) という2つの演算や、salaries = で始まる2つの異なる行、そしていくつかのリスト内包表記などが挙げられます。
そこで、このコードをAmazing New Project™ の一部で使用する予定なので、これらの問題を修正したいと考えています。
しかし、このクラスは完全に動作しています。そのため、私たちの作業はリファクタリングでなければなりません。つまり、以前のオブジェクトの動作を維持しながら、より良いものを書きたいということです。
開発方法
この記事では、テストを使ってこのようなクラスを安全にリファクタリングする方法を紹介したいと思います。これはTDDとは異なりますが、この2つは密接に関連しています。今あるクラスはテストがないのでTDDで作られたものではありませんが、テストを使ってその挙動を確実に維持することができます。したがって、これはテスト駆動型リファクタリング（TDR: Test Driven Refactoring）と呼ばれるべきものです。
TDRの考え方はとてもシンプルです。まず、あるコードの振る舞いをチェックするテストを書かなければなりません。おそらく、範囲と出力が明確に定義された小さな部分です。これは死後(または晩年)のユニットテストであり、コードの作者が提供すべきものをシミュレートします(数ヶ月前のあなたです)。
この単体テストがあれば、結果として得られるオブジェクトの振る舞いが以前のものと同じであることを知った上で、コードを修正することができます。容易に理解できるように、この方法論の有効性はテスト自体の品質に強く依存しており、おそらくTDDで開発する場合よりも高く、これがリファクタリングが難しい理由です。
注意点
リファクタリングを始める前に2つの注意点があります。1つ目は、このようなクラスは簡単に機能的なコードにリファクタリングできるということです。最終的な結果から推測できるように、このコードにオブジェクト指向のアプローチを維持する本当の理由はありません。しかし、ラッパーと呼ばれるデザインパターンと、それを利用したリファクタリング技術を紹介できる可能性があったので、この方法を取ることにしました。
2つ目の注意点は、純粋なTDDでは、内部メソッド、つまりオブジェクトのパブリックAPIを形成していないメソッドをテストしないことを強くお勧めします。一般的に、Pythonではそのようなメソッドは名前の前にアンダースコアを付けることで識別します。それらをテストしない理由は、TDDがオブジェクトを構造ではなく動作として考えるオブジェクト指向プログラミングの方法論に従ってオブジェクトを形成することを望んでいるからです。したがって、パブリックメソッドのテストにしか興味がありません。
しかし、公開したくないメソッドであっても、そのメソッドにはテストしたい複雑なロジックが含まれていることがあるのも事実です。ですから、私の考えでは、TDDのアドバイスは、「内部メソッドをテストするのは、それが自明でないロジックを含んでいるときだけにしてください」というようなものです。
しかし、リファクタリングとなると、既存の構造を解体することになるので、コードの一部を抽出して一般化するために、通常は多くのプライベートメソッドを作成することになります。この場合、私がアドバイスするのは、これらのメソッドをテストすることです。なぜなら、そうすることで、自分のやっていることに高い自信を持つことができるからです。経験を積めば、どのテストが必要で、どのテストが必要でないかがわかってくるでしょう。
テスト環境の構築
リポジトリ をクローンして、仮想環境を作成します。このリポジトリを有効にして、必要なパッケージをインストールしてください。 code: bash
$ cd datastats
$ piip install -r requirements.txt
リポジトリにはすでに pytest 用の設定ファイルがありますが、仮想環境のディレクトリを入力しないようにカスタマイズする必要があります。このファイルの norecursedirs パラメータを修正し、先ほど作成した仮想環境の名前を追加してください。私は通常、仮想環境の名前に venv というプレフィックスを付けているため、この変数に venv*. というエントリが含まれています。
この時点で、リポジトリの親ディレクトリ（pytest.ini があるディレクトリ）で pytest -svv を実行すると、以下のような結果が得られるはずです。
code: bash
========================== test session starts ==========================
platform linux -- Python 3.5.3, pytest-3.1.2, py-1.4.34, pluggy-0.4.0
cachedir: .cache
rootdir: datastats, inifile: pytest.ini
plugins: cov-2.5.1
collected 0 items
====================== no tests ran in 0.00 seconds ======================
master はあなたが入っているブランチで、初期設定が含まれており、develop はリファクタリングプロセス全体の最後のステップを指しています。この記事の各ステップには、そのセクションで導入した変更を含むコミットへの参照が含まれています。
ステップ1 - エンドポイントのテスト
システムのリファクタリングを始めるときには、規模の大小にかかわらず、エンドポイントをテストしなければなりません。これは、システムをブラックボックス(内部に何があるかわからない)とみなして、外部の動作をチェックすることを意味します。この場合、クラスを初期化し、stats()メソッドをいくつかのテストデータ（おそらく実際のデータ）で実行し、その出力をチェックするテストを書くことができます。もちろん、メソッドが返す実際の出力を使ってテストを書きますので、このテストは自動的にパスします。
サーバーに問い合わせたところ、次のようなデータが得られました。
code: JSON
test_data = [
{
"id": 1,
"name": "Laith",
"surname": "Simmons",
"age": 68,
"salary": "£27888"
},
{
"id": 2,
"name": "Mikayla",
"surname": "Henry",
"age": 49,
"salary": "£67137"
},
{
"id": 3,
"name": "Garth",
"surname": "Fields",
"age": 70,
"salary": "£70472"
}
]
iageを20に、isalaryを20000に設定して、この出力を使ってstats()メソッドを呼び出すと、次のようなJSONの結果が得られます。
code: JSON
{
"avg_age": 62,
"avg_salary": 55165,
"avg_yearly_increase": 837,
"max_salary": [{
"id": 3,
"name": "Garth",
"surname": "Fields",
"age": 70,
"salary": "£70472"
}],
"min_salary": [{
"id": 1,
"name": "Laith",
"surname": "Simmons",
"age": 68,
"salary": "£27888"
}]
}
注意：3つの辞書のリストという、非常に短い実データを使っています。実際のケースでは、ブラックボックスを様々なユースケースでテストし、コーナーケース(Coner Case)をチェックしていないことを確認します。
訳注：コーナーケース(Coner Case)
コーナーケースとは、場合分けの境界などの特定の条件を満たすテストケースのことです。例えば境界値での条件分岐の正しさを確認するものをいいます。適当な入力に対しては正しい出力ができても、コーナーケースに対しては間違った出力になる場合があります。
もっとわかりやすく言い換えると、 「ミスの発生しやすそうなところを突くためのテストケース」です。
テストは以下の通りです。
code: python
import json
from datastats.datastats import DataStats
def test_json():
test_data = [
{
"id": 1,
"name": "Laith",
"surname": "Simmons",
"age": 68,
"salary": "£27888"
},
{
"id": 2,
"name": "Mikayla",
"surname": "Henry",
"age": 49,
"salary": "£67137"
},
{
"id": 3,
"name": "Garth",
"surname": "Fields",
"age": 70,
"salary": "£70472"
}
]
ds = DataStats()
assert ds.stats(test_data, 20, 20000) == json.dumps(
{
'avg_age': 62,
'avg_salary': 55165,
'avg_yearly_increase': 837,
'max_salary': [{
"id": 3,
"name": "Garth",
"surname": "Fields",
"age": 70,
"salary": "£70472"
}],
'min_salary': [{
"id": 1,
"name": "Laith",
"surname": "Simmons",
"age": 68,
"salary": "£27888"
}]
}
)
前述のように、このテストは明らかに合格しています。実際のコードの実行から人工的に作られたものです。
さて、このテストは非常に重要です。これで、コード内で何かを変更してクラスの動作を変えた場合、少なくとも1つのテストが失敗することがわかりました。
ステップ2 - JSONフォーマットを取り除く
このメソッドは出力をJSON形式で返しますが、クラスを見るとjson.dumps() によって変換が行われていることがよくわかります。
コードの構造は以下のようになっています。
code: python
class DataStats:
def stats(self, data, iage, isalary):
return json.dumps({
})
明らかにcode_part_2はcode_part_1に依存しています。最初のリファクタリングは、次のような手順で行われます。
1. test__stats()というテストを、Pythonの構造体としてデータを返すはずの _stats() メソッドに対して書きます。後者は、JSONから手動で推測するか、Pythonシェルから json.loads() を実行します。このテストは失敗します。
2. データを生成する stats() メソッドのコードを複製して、新しい _stats() メソッドに入れます。このテストは合格です。
code: python
class DataStats:
def _stats(parameters):
def stats(self, data, iage, isalary):
return json.dumps({
})
3. stats() 内の重複したコードを削除し、_stats() への呼び出しに置き換えます。
code: python
class DataStats:
def _stats(parameters):
def stats(self, data, iage, isalary):
return json.dumps(
self._stats(data, iage, isalary)
)
この時点で、最初に書いたテスト test_json() をリファクタリングすることもできますが、これは高度な検討事項なので、後のメモに残しておきます。
さて、このクラスのコードは次のようになっています。
code: python
class DataStats:
def _stats(self, data, iage, isalary):
# iage and isalary are the starting age and salary used to
# compute the average yearly increase of salary.
# Compute average yearly increase
average_age_increase = math.floor(
sum([e'age' for e in data])/len(data)) - iage average_salary_increase = math.floor(
sum([int(e'salary'1:) for e in data])/len(data)) - isalary yearly_avg_increase = math.floor(
average_salary_increase/average_age_increase)
# Compute max salary
threshold = '£' + str(max(salaries))
max_salary = [e for e in data if e'salary' == threshold] # Compute min salary
min_salary = [e for e in data if e'salary' == '£{}'.format(str(min(salaries)))]
return {
'avg_age': math.floor(sum([e'age' for e in data])/len(data)), 'avg_salary': math.floor(sum(
'avg_yearly_increase': yearly_avg_increase,
'max_salary': max_salary,
'min_salary': min_salary
}
def stats(self, data, iage, isalary):
return json.dumps(
self._stats(data, iage, isalary)
)
そして、それが正しいかどうかをチェックする2つのテストがあります。
ステップ3 - テストのリファクタリング
辞書のリスト test_data は、実行するすべてのテストで使用されることが明らかになっているので、そろそろグローバル変数に移すことにしましょう。テストデータは単なる静的なデータなので、フィクスチャを使う意味はありません。
出力データをグローバル変数に移すこともできますが、今後のテストではもう出力辞書全体を使うことはないので、決断を先延ばしにすることができます。
テスト・スイートは以下のようになります。
code: python
import json
from datastats.datastats import DataStats
test_data = [
{
"id": 1,
"name": "Laith",
"surname": "Simmons",
"age": 68,
"salary": "£27888"
},
{
"id": 2,
"name": "Mikayla",
"surname": "Henry",
"age": 49,
"salary": "£67137"
},
{
"id": 3,
"name": "Garth",
"surname": "Fields",
"age": 70,
"salary": "£70472"
}
]
def test_json():
ds = DataStats()
assert ds.stats(test_data, 20, 20000) == json.dumps(
{
'avg_age': 62,
'avg_salary': 55165,
'avg_yearly_increase': 837,
'max_salary': [{
"id": 3,
"name": "Garth",
"surname": "Fields",
"age": 70,
"salary": "£70472"
}],
'min_salary': [{
"id": 1,
"name": "Laith",
"surname": "Simmons",
"age": 68,
"salary": "£27888"
}]
}
)
def test__stats():
ds = DataStats()
assert ds._stats(test_data, 20, 20000) == {
'avg_age': 62,
'avg_salary': 55165,
'avg_yearly_increase': 837,
'max_salary': [{
"id": 3,
"name": "Garth",
"surname": "Fields",
"age": 70,
"salary": "£70472"
}],
'min_salary': [{
"id": 1,
"name": "Laith",
"surname": "Simmons",
"age": 68,
"salary": "£27888"
}]
}
ステップ4 - 平均年齢のアルゴリズムを分離する
独立した機能を分離することは、ソフトウェア設計の重要な目標です。そのため、今回のリファクタリングでは、コードを分離し、小さな独立した関数に分割することを目指します。
出力辞書には5つのキーがあり、それぞれが計算された値（avg_ageとavg_salaryの場合）、またはメソッドのコードによって計算された値（avg_yearly_increase、max_salary、min_salaryの場合）に対応しています。アルゴリズムを分離するために、各キーの値を計算するコードを専用のメソッドに置き換えていきます。
コードを分離するためには、まずそのコードを複製して、専用のメソッドに入れることです。テストを使ってリファクタリングを行っているので、まず最初にこのメソッドのテストを書きます。
code: python
def test__avg_age():
ds = DataStats()
assert ds._avg_age(test_data) == 62
元の stats() メソッドの出力データには 62 という値があるので、このメソッドの出力は 62 であることがわかります。なお、iage と isalary はリファクタリング後のコードでは使用されていないので、渡す必要はありません。
テストが失敗したので、avg_age を計算するためのコードを複製してみましょう。
code: python
def _avg_age(self, data):
return math.floor(sum([e'age' for e in data])/len(data)) そして、テストに合格したら、_stats() の重複したコードを _avg_age() の呼び出しに置き換えることができます。
code: python
return {
'avg_age': self._avg_age(data),
'avg_salary': math.floor(sum(
'avg_yearly_increase': yearly_avg_increase,
'max_salary': max_salary,
'min_salary': min_salary
}
その後、どのテストも失敗していないことを確認します。よくできました。最初の機能を分離し、リファクタリングによって3つのテストを作成しました。
ステップ5 - 平均給与アルゴリズムの分離
avg_salaryキーはavg_ageと同じように動作しますが、コードは異なります。したがって、リファクタリングのプロセスは以前と同じで、結果は新しい test__avg_salary() テストになります。
code: python
def test__avg_salary():
ds = DataStats()
assert ds._avg_salary(test_data) == 55165
そして、新しい _avg_salary() は次のようになります。
code: python
def _avg_salary(self, data):
return math.floor(sum([int(e'salary'1:) for e in data])/len(data)) そして、これが最終的な戻り値の新しいバージョンです。
code: python
return {
'avg_age': self._avg_age(data),
'avg_salary': self._avg_salary(data),
'avg_yearly_increase': yearly_avg_increase,
'max_salary': max_salary,
'min_salary': min_salary
}
ステップ6 - 平均年間増加率のアルゴリズムを分離する
残りの3つのキーは、アルゴリズムで計算されていますが、1行以上の長さがあるため、辞書の定義に直接押し込むことができませんでした。しかし、リファクタリングのプロセスは特に変わりません。以前のように、まずヘルパー・メソッドをテストし、次にそれを定義してコードを重複させ、最後にヘルパーを呼び出してコードの重複を取り除きます。
給料の平均年収については、新しいテストを行います。
code: python
def test__avg_yearly_increase():
ds = DataStats()
assert ds._avg_yearly_increase(test_data, 20, 20000) == 837
テストにパスした新しいメソッド
code: python
def _avg_yearly_increase(self, data, iage, isalary):
# iage and isalary are the starting age and salary used to
# compute the average yearly increase of salary.
# Compute average yearly increase
average_age_increase = math.floor(
sum([e'age' for e in data])/len(data)) - iage average_salary_increase = math.floor(
sum([int(e'salary'1:) for e in data])/len(data)) - isalary return math.floor(average_salary_increase/average_age_increase)
新しいバージョンの_stas()メソッド
code: python
def _stats(self, data, iage, isalary):
# Compute max salary
threshold = '£' + str(max(salaries))
max_salary = [e for e in data if e'salary' == threshold] # Compute min salary
min_salary = [e for e in data if e'salary' == '£{}'.format(str(min(salaries)))]
return {
'avg_age': self._avg_age(data),
'avg_salary': self._avg_salary(data),
'avg_yearly_increase': self._avg_yearly_increase(
data, iage, isalary),
'max_salary': max_salary,
'min_salary': min_salary
}
コードの重複を解決するのではなく、リファクタリングのために導入するものであることに注意してください。最初に目指すべき成果は、独立した機能を完全に分離することです。
ステップ7 - 給与の最大値と最小値のアルゴリズムを分離する
リファクタリングを行う際には、常に一度に一つのことを行わなければなりませんが、簡潔にするために、ここでは二つのリファクタリングステップの結果を一度に示します。以下に掲載するコードを書いたときのように、独立したステップとして実行することを読者にお勧めします。
新しいテストは次になります。
code: python
def test__max_salary():
ds = DataStats()
assert ds._max_salary(test_data) == [{
"id": 3,
"name": "Garth",
"surname": "Fields",
"age": 70,
"salary": "£70472"
}]
def test__min_salary():
ds = DataStats()
assert ds._min_salary(test_data) == [{
"id": 1,
"name": "Laith",
"surname": "Simmons",
"age": 68,
"salary": "£27888"
}]
DataStatsクラスの新しいメソッドは次のようになります。
code: python
def _max_salary(self, data):
# 最高給与額の計算
threshold = '£' + str(max(salaries))
return [e for e in data if e'salary' == threshold] def _min_salary(self, data):
# Compute min salary
'£{}'.format(str(min(salaries)))]
ステップ8 - コードの重複を減らす
メインのテストができたので、ヘルパーメソッドのコードを変更してみましょう。これで、テストをしなくてもコードを変更できる程度にはなりました。しかし、一般的には「十分に小さい」とは何を意味するのか、 「ユニットテスト」とは何なのかという定義はありません。一般的には、あなたが行っている変更が、あなたが持っているテストでカバーされていることに自信を持つべきです。そうでない場合は、十分な自信が持てるようになるまで 1 つ以上のテストを追加したほうがいいでしょう。
2つのメソッド _max_salary() と _min_salary() は、2つ目のメソッドの方がより簡潔であるにもかかわらず、大量のコードを共有しています。
code: python
def _max_salary(self, data):
# Compute max salary
threshold = '£' + str(max(salaries))
return [e for e in data if e'salary' == threshold] def _min_salary(self, data):
# Compute min salary
'£{}'.format(str(min(salaries)))]
まずは、2つ目の関数で閾値の変数を明示することから始めます。何かを変更したら、すぐにテストを実行して、外部の動作に変化がないことを確認します。
code: python
def _max_salary(self, data):
# 高給与額の計算
threshold = '£' + str(max(salaries))
return [e for e in data if e'salary' == threshold] def _min_salary(self, data):
# Compute min salary
threshold = '£{}'.format(str(min(salaries)))
return [e for e in data if e'salary' == threshold] さて、2つの関数が同じものであることは一目瞭然ですが、min() と max() の関数が違います。min() と max() 以外は同じですが、変数名としきい値のフォーマットが異なるため、まず最初に、_min_salary() のコードを _max_salary() にコピーし、min() を max() に変更します。
code: python
def _max_salary(self, data):
# Compute max salary
threshold = '£{}'.format(str(max(salaries)))
return [e for e in data if e'salary' == threshold] def _min_salary(self, data):
# Compute min salary
threshold = '£{}'.format(str(min(salaries)))
return [e for e in data if e'salary' == threshold] 次に、_select_salary() というヘルパー関数を作ります。このヘルパーは、コードを複製し、min() や max() の代わりに関数を受け取ることができます。先ほどと同じように、まずコードを複製し、次に新しい関数を呼び出して複製を削除します。
いくつかの通過点を経て、コードは次のようになります。
code: python
def _select_salary(self, data, func):
threshold = '£{}'.format(str(func(salaries)))
return [e for e in data if e'salary' == threshold] def _max_salary(self, data):
return self._select_salary(data, max)
def _min_salary(self, data):
return self._select_salary(data, min)
その後、_avg_salary() と _select_salary() の間でコードが重複していることに気づきました。
code: python
def _avg_salary(self, data):
return math.floor(sum([int(e'salary'1:) for e in data])/len(data)) code: python
def _select_salary(self, data, func):
_salaries()というメソッドで共通のアルゴリズムを抽出することにしました。以前のように、まずテストを書きます。
code: python
def test_salaries():
ds = DataStats()
そして、メソッドを実装します。
code: python
def _salaries(self, data):
最終的には重複したコードを新しいメソッドの呼び出しに置き換えます。
code: python
def _salaries(self, data):
code: python
def _select_salary(self, data, func):
threshold = '£{}'.format(str(func(self._salaries(data))))
return [e for e in data if e'salary' == threshold] この作業中に、_avg_yearly_increase() にも同じコードが含まれていることに気づき、そちらも修正しました。
code: python
def _avg_yearly_increase(self, data, iage, isalary):
# iage and isalary are the starting age and salary used to
# compute the average yearly increase of salary.
# Compute average yearly increase
average_age_increase = math.floor(
sum([e'age' for e in data])/len(data)) - iage average_salary_increase = math.floor(
sum(self._salaries(data))/len(data)) - isalary
return math.floor(average_salary_increase/average_age_increase)
この時点で、入力データをクラス内に保存し、クラスのすべてのメソッドに渡す代わりに、self.dataとして使用することができれば便利です。しかし、DataStatsは現在データなしで初期化されているため、これはクラスのAPIを壊すことになります。後で、APIを壊す可能性のある変更を導入する方法を示し、この問題について簡単に説明します。しかし、今のところ、外部インターフェイスを変更せずにクラスを変更し続けます。
age は salary と同じようにコードの重複があるようなので、同じ手順で _ages() メソッドを導入し、それに合わせて_avg_age() メソッドと _avg_yearly_increase() メソッドを変更します。
_avg_yearly_increase() について言えば、このメソッドのコードには _avg_age() と _avg_salary() メソッドのコードが含まれているので、2つの呼び出しに置き換える価値があります。既存のメソッド間でコードを移動させているので、さらなるテストは必要ありません。
code: python
def _avg_yearly_increase(self, data, iage, isalary):
# iage and isalary are the starting age and salary used to
# compute the average yearly increase of salary.
# Compute average yearly increase
average_age_increase = self._avg_age(data) - iage
average_salary_increase = self._avg_salary(data) - isalary
return math.floor(average_salary_increase/average_age_increase)
ステップ9 - 高度なリファクタリング
最初のクラスは __init__() メソッドを持っていなかったので、オブジェクト指向のパラダイムのカプセル化の部分が欠けていました。stats() メソッドは簡単に抽出でき、プレーンな関数として提供できたので、このクラスを維持する理由はありませんでした。
データをパラメータとして受け取るメソッドが10個もあるので、リファクタリングを行った今では、このことがより明確になっています。インスタンス化の際に入力データをクラスに読み込み、self.data としてアクセスできるといいですね。そうすれば、クラスの可読性が大幅に向上し、クラスの存在が正当化されます。
しかし、パラメータを必要とする __init__() メソッドを導入すると、クラスのAPIを変更することになり、クラスをインポートして使用しているコードとの互換性が失われてしまいます。このクラスを残したいのであれば、新しくてきれいなクラスの利点と、安定したAPIの利点の両方を提供する方法を考案しなければなりません。これはいつも完璧に実現できるわけではありませんが、今回のケースでは Adapter デザインパターン (Wrapper としても知られています) がこの問題を完璧に解決してくれます。
目標は、現在のクラスを新しい API に合わせて変更し、最初のクラスをラップして古い API を提供するクラスを構築することです。戦略は以前行ったものとそれほど変わりませんが、今回はメソッドではなくクラスを扱うことになります。想像力を働かせて、新しいクラスをNewDataStatsと名付けました。申し訳ありませんが、時には仕事をこなさなければなりません。
リファクタリングではよくあることですが、まずコードを複製し、新しいコードを挿入するときには、それを正当化するテストが必要です。新しいクラスは以前のクラスと同じ機能を提供するので、テストは以前と同じになります。そこで、test_newdatastats.py という新しいファイルを作成して、最初の test_init() を置いていきます。
code: python
import json
from datastats.datastats import NewDataStats
test_data = [
{
"id": 1,
"name": "Laith",
"surname": "Simmons",
"age": 68,
"salary": "£27888"
},
{
"id": 2,
"name": "Mikayla",
"surname": "Henry",
"age": 49,
"salary": "£67137"
},
{
"id": 3,
"name": "Garth",
"surname": "Fields",
"age": 70,
"salary": "£70472"
}
]
def test_init():
ds = NewDataStats(test_data)
assert ds.data == test_data
このテストはパスしませんが、このクラスを実装したコードは非常にシンプルです。
code: python
class NewDataStats:
def __init__(self, data):
self.data = data
これで、反復プロセスを始めることができます。
1. DataStatsのテストの1つをコピーし、それをNewDataStatsに適応させます。
2. DataStatsからNewDataStatsにコードをコピーして、新しいAPIに適合させ、テストを通過させます。
この時点で、DataStatsからメソッドを繰り返し削除して、NewDataStatsの呼び出しに置き換えるのはやりすぎです。その理由と、それを回避するためにできることを次のセクションで紹介します。
NewDataStatsのテストの例は以下の通りです。
code: python
def test_ages():
ds = NewDataStats(test_data)
そして、テストをパスしたコードです。
code: python
def _ages(self):
return [d'age' for d in self.data] 完成後、_ages() のようなメソッドが入力パラメータを必要としなくなったので、それらをプロパティに変換し、それに応じてテストを変更することができることに気づきました。
code: python
@property
def _ages(self):
return [d'age' for d in self.data] DataStatsのメソッドを、NewDataStatsの呼び出しに置き換える時が来ました。メソッドごとに置き換えることもできますが、実際に必要なのはstats()を置き換えることだけです。ですから、新しいコードが次です。
code: python
class DataStats:
def stats(self, data, iage, isalary):
nds = NewDataStats(data)
return nds.stats(iage, isalary)
他のメソッドはもう使われていないので、テストが失敗しないことを確認しながら安全に削除することができます。テストといえば、メソッドを削除するとDataStatsの多くのテストが失敗するので、それらを削除する必要があります。
ステップ10 - まだまだ改善の余地あり
リファクタリングは反復的なプロセスなので、可能な限りのことをやったと思っても、後で何かを見逃していたことに気づくことがよくあります。今回のケースでは、Harun Yasar氏が見落としていたステップを発見し、さらに小さなコードの重複に気づきました。
2つの関数は同じロジックを共有しています。
code: python
def _avg_salary(self):
return math.floor(sum(self._salaries)/len(self.data))
def _avg_age(self):
return math.floor(sum(self._ages)/len(self.data))
そこを確実に分離して、それぞれの関数で共通のコードを呼び出すことができます。
code: python
def _floor_avg(self, sum_of_numbers):
return math.floor(sum_of_numbers / len(self.data))
def _avg_salary(self):
return self._floor_avg(sum(self._salaries))
def _avg_age(self):
return self._floor_avg(sum(self._ages))
これは、すべてのテストにパスしているので、正しいです。
私の記事を読んで何か新しいことを学んだという人からコメントをもらうと、とても嬉しくなります。
最後に
リファクタリング・セッションの小さなツアーが、あまりにもつまらない結果にならず、このテクニックの基本的なコンセプトを理解するのに役立ったことを願っています。このテーマに興味をお持ちの方には、Martin Fowler の古典的な書籍 "Refactoring: Improving the Design of Existing Code"のパターンを集めたものです。この書籍でのリファレンス言語はJavaですが、コンセプトはPythonにも簡単に適応できます。
参考