[JAVA] code smell refactoring [feature envy]

The third refactoring is feature envy.

What is feature envy

A function in one module interacts with a function or data structure in an external module rather than an internal module.

For example, a common example is when you are using a chain of get methods from other modules.

Bad code example

For example, see the code below.

From the House class, we follow the Person class Adress class and call the calcurateDistance method of the ʻAdress class`.

Obviously, the House class knows too much about other classes and references them too much.

House().getPerson().getAdress().calcurateDistance()

Why bad

So why is such an excessive reference to an external module bad in the first place?

In the first place, when designing well, it is good to interact well with the internal module and minimize the interaction with the external module.

By doing this,

By combining the processes, the degree of cohesion increases, and even if it is changed, the effect is reduced.

solution

There are two main possible solutions:

--Move function

--Move field value

Let's actually look at a code example.

In this code example, there are four models. House Price Address SalesPerson

Suppose you have a program that calculates the price of a house or the distance to a house.

Before refactoring

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
    }
  }
 
}

There seems to be smell in the SalesPerson class.

If you take a closer look, both the calculatePrice and calculateDistance methods refer to the field values in the house class.

It's a typical feature envy smell. That is, you refer to the field values of the external module more often than your own field values. </ b>

We will refactor this.

Which is more suitable for refactoring in this case, moving field values or moving functions?

Move field values,

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
    }
  }

The smell in the two functions may be gone, but the cohesion of the model is broken.

The SlaesPerson class will end up with irrelevant data such as address price.

Let's move the function obediently.

After refactoring



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
                        )

By moving the two functions to the House class, the feature envy smell disappeared, the behavior of the House class increased, and the cohesion increased.

Summary

As my feeling If you're trying to access the data in the next module or even the next module, you should suspect it's a feature envy.

Is the location where the function is defined different, as in the refactoring above, or is it better to move the field values of the object?

Thank you until the end.

Next time, I will write an article about lazy class.

reference

Recommended Posts

code smell refactoring [feature envy]
code smell refactoring [lazy class]
code smell Refactoring 1st [long method]