はじめに
表題のリファクタリング手法を学んだので、今回はこのことについて記事にしてみたいと思います。
リファクタリング前
まずはこちらのコードを見てください。
void equipArmor(int menberId, Equipment newArmor) {
if (party.members.get(menberId).equipments.canChange) {
party.members.get(menberId).equipments.armor = newArmor;
}
}
一見するとよくあるメソッドチェインを介したシンプルな処理に思えます。
しかしこのコードには以下のような問題点があります。
- 目的の処理を実装するために長大なメソッドチェインが必要になっている
- これら諸要素へのアクセスを介する点で互いに深く依存している
- それ故に状態遷移の見通しが悪くリグレッション範囲が大規模化しかねない
別の言い方をすると、
- 関心事の分離がなされていないが故に状態が連鎖的に他者に漏れている
- 多くの内部構造を知っていることが前提になったコンポーネントである
このように言い換えられます。
詰まる所、
- 単一責任に基づくクラス設計がなされていない
- カプセル化が効いておらず、状態漏れによる可読性、メンテナンス性、変更容易性が悪化している
このような状態に陥っているのです。
「.」の数それそのものというよりも、それが状態参照の連続である、ということ、
それに依存している、ということが問題なわけです。
そして、このような単一責任化されていないコンポーネントは色んなところにまばらに実装されます。
そうなればコンポーネントの冗長化もそうですが、それ以上に状態遷移のタイミングが追いかけづらくなり、拡張性の低いコードが生まれてしまいます。
加えて、上記のようなコードだと内部構造を深く知ってしまっているため、自動テストを実装する際に依存性の注入で足枷になってしまいますし、
参照先のコードの振る舞いの変化に依存してしまうことで、偽陽性や偽陰性の発生がしやすくなってしまいます。
詳細については僭越ながら以下の記事をご参照ください。
これらにより、プロダクトコード、およびプロダクトの持続的成長に悪影響を及ぼしつつ、
リグレッションテストの効率が悪くなり、開発プロセスにも悪影響が及びます。
リファクタリング後
このような状態にあるコードですが、以下のようにリファクタリングを施すことで状況を改善できます。
class Equipments {
Equipments({
required bool canChange,
required Equipment head,
required Equipment armor,
required Equipment arm,
}) : _canChange = canChange,
_head = head,
_armor = armor,
_arm = arm;
bool _canChange;
Equipment _head;
Equipment _armor;
Equipment _arm;
void equipArmor(Equipment newArmor) {
if (_canChange) {
_armor = newArmor;
}
}
void deactivateAll() {
_head = Equipment.EMPTY;
_armor = Equipment.EMPTY;
_arm = Equipment.EMPTY;
}
}
先述のequipArmorをEquipmentsクラスの関心事として定義することで、
「防具の変化」という状態遷移がこのコンポーネントでのみ完結するようになりました。
プロパティとその状態の参照・更新がセットになっており、カプセル化のなされたクラスが設計されています。
これにより外部からは必ずEquipmentsクラスを介するようになるので意図しない状態の変化が起こりづらくなります。
参照元へはその効能だけを提供することで状態の参照・更新が安全にできるようになる、ということです。
また、状態の参照がEquipmentsクラスのオブジェクトにのみ閉じることになるので、影響範囲も限定化することができます。
それは自動テストの実装の際にもプラスに働き、最低限の依存性の注入でのみその振る舞いを保証することができるようになり、
「テストのテストをする」といったあべこべなことも起こりにくくなります。
参考