code smellリファクタリング[feature envy編]

第三回のリファクタリングは、feature envyです。

## feature envyとは

あるモジュールの関数が、内部のモジュールよりも、外部のモジュールの関数やデータ構造とやりとりしているということ。


例えば、よくある例としては、他のモジュールのgetメソッドをチェーンして使っている場合など。


### 悪いコード例
例えば、以下のコードを見てください。

`Houseクラス`から、`Personクラス Adressクラス`を辿って、`Adressクラス`の`calcurateDistanceメソッド`を呼び出しています。

明らかに、Houseクラスは他のクラスを知りすぎており、参照しすぎています。

```scala
House().getPerson().getAdress().calcurateDistance()
```

## なぜ悪いのか

では、そもそもなぜこのような過度の外部モジュールへの参照が悪いのでしょうか？

そもそも良い設計をする時に、内部モジュールとよくやりとりし、外部モジュールとのやりとりは最小限にすることが良いとされています。

こうすることで、

#### 処理をまとめることで凝集度が上がり、変更しても影響が少なくなる。


## ソリューション

主に考えられるソリューションは以下の二つがあります。


  - 関数の移動

  - フィールド値の移動


では実際に、コード例を見ていきます。

このコード例では、四つのモデルがあります。
House Price Address SalesPerson

家の値段を計算したり、家までの距離を計算したりするプログラムだと仮定します。



## リファクタリング前
```scala
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ですね。つまり、<b>自分のフィールド値よりも外部モジュールのフィールド値をよく参照しています。</b>

これを、リファクタリングしていきます。


この場合のリファクタリングには、フィールド値の移動・関数の移動のどちらが適しているでしょうか。

フィールド値の移動を行う、

```scala
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などの関係のないデータを持つようになってしまいます。

これは、素直に関数の移動をしてあげましょう。


## リファクタリング後

```scala


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に関しての記事を書きます。


## 参考

- [clean code](https://amzn.to/38GalcX)
- [リファクタリング](https://amzn.to/38KuqPB)