第三回のリファクタリングは、feature envyです。
feature envyとは
あるモジュールの関数が、内部のモジュールよりも、外部のモジュールの関数やデータ構造とやりとりしているということ。
例えば、よくある例としては、他のモジュールのgetメソッドをチェーンして使っている場合など。
悪いコード例
例えば、以下のコードを見てください。
Houseクラスから、Personクラス Adressクラスを辿って、AdressクラスのcalcurateDistanceメソッドを呼び出しています。
明らかに、Houseクラスは他のクラスを知りすぎており、参照しすぎています。
House().getPerson().getAdress().calcurateDistance()
なぜ悪いのか
では、そもそもなぜこのような過度の外部モジュールへの参照が悪いのでしょうか？
そもそも良い設計をする時に、内部モジュールとよくやりとりし、外部モジュールとのやりとりは最小限にすることが良いとされています。
こうすることで、
処理をまとめることで凝集度が上がり、変更しても影響が少なくなる。
ソリューション
主に考えられるソリューションは以下の二つがあります。
-
関数の移動
-
フィールド値の移動
では実際に、コード例を見ていきます。
このコード例では、四つのモデルがあります。
House Price Address SalesPerson
家の値段を計算したり、家までの距離を計算したりするプログラムだと仮定します。
リファクタリング前
class Main {
def main(args: Array[String]): Unit = {
val address = Address(1L, 1L, 100L)
val price = Price(10000)
val house = House(address, price)
val salesPerson = SalesPerson(1, "kin", house)
salesPerson.calculatePrice
}
case class House(address: Address, price: Price)
case class Price(number: Int)
case class Address(
longitude: Long,
latitude: Long,
price: Long
)
case class SalesPerson(
id: Int,
name: String,
house: House
) {
def calculatePrice: Int = {
(house.price.number * 1.1).toInt
}
def calculateDistance: Long = {
house.address.longitude
}
}
}
SalesPersonクラスにsmellがありそうですね。
よく見てみると、calculatePriceメソッドとcalculateDistanceメソッドの両方が、houseクラスのフィールド値を参照しています。
典型的なfeature envy smellですね。つまり、自分のフィールド値よりも外部モジュールのフィールド値をよく参照しています。
これを、リファクタリングしていきます。
この場合のリファクタリングには、フィールド値の移動・関数の移動のどちらが適しているでしょうか。
フィールド値の移動を行う、
case class House() {
}
case class SalesPerson(id: Int, name: String, house: House, address: Address, price: Price) {
def calculatePrice: Int = {
(price.number * 1.1).toInt
}
def calculateDistance: Long = {
address.longitude
}
}
二つの関数内のsmellは無くなったかもしれませんが、modelの凝集度が破綻していますね。
SlaesPersonクラスが、address priceなどの関係のないデータを持つようになってしまいます。
これは、素直に関数の移動をしてあげましょう。
リファクタリング後
def main(args: Array[String]): Unit = {
val address = Address(1L, 1L, 100L)
val price = Price(10000)
val house = House(address, price)
val salesPerson = SalesPerson(1, "kin", house)
// salesPerson.calculatePrice
house.calculatePrice
house.calculateDistance
}
case class House(address: Address, price: Price) {
def calculatePrice: Int = {
(price.number * 1.1).toInt
}
def calculateDistance: Long = {
address.longitude
}
}
case class SalesPerson(
id: Int,
name: String,
house: House
)
二つの関数をHouseクラスに移動させることで、feature envy smellは消え、さらにHouseクラスの振る舞いも増えて凝集度が高まりました。
まとめ
僕の感覚として、
隣のさらにまた隣のモジュールのデータにまでアクセスしようとしているなら、それはfeature envyだと疑った方がいいかと思います。
上記のリファクタリングのように、関数を定義する場所が違うのか、それとも、オブジェクトのフィールド値を移動させた方がいいかと。
最後までありがとうございました。
次回は、lazy classに関しての記事を書きます。