Feature envy: move logic to its data
Put a calculation next to the data it reads, so it lives once.
Watch the narrated video
Every step
Problem
Steps 1–3Refactor
Steps 4–7Payoff
Step 8Recap
Steps 9–10The code: before and after
Before
class Parcel:
def __init__(self, name: str, weight_kg: int,
length_cm: int, width_cm: int,
height_cm: int) -> None:
self._name = name
self._weight_kg = weight_kg
self._length_cm = length_cm
self._width_cm = width_cm
self._height_cm = height_cm
@property
def name(self) -> str:
return self._name
@property
def weight_kg(self) -> int:
return self._weight_kg
@property
def length_cm(self) -> int:
return self._length_cm
@property
def width_cm(self) -> int:
return self._width_cm
@property
def height_cm(self) -> int:
return self._height_cm
class ShippingQuote:
def __init__(self, rate_per_kg: int) -> None:
self._rate_per_kg = rate_per_kg
def price(self, parcel: Parcel) -> str:
volume = (parcel.length_cm * parcel.width_cm
* parcel.height_cm)
billable_kg = max(parcel.weight_kg, volume // 5000)
cost = billable_kg * self._rate_per_kg
return f"{parcel.name}: {cost} USD"
def main() -> None:
boots = Parcel("Boots", 3, 50, 40, 30)
books = Parcel("Books", 8, 20, 20, 10)
quote = ShippingQuote(2)
print(quote.price(boots))
print(quote.price(books))After
class Parcel:
def __init__(self, name: str, weight_kg: int,
length_cm: int, width_cm: int,
height_cm: int) -> None:
self._name = name
self._weight_kg = weight_kg
self._length_cm = length_cm
self._width_cm = width_cm
self._height_cm = height_cm
@property
def name(self) -> str:
return self._name
def billable_kg(self) -> int:
volume = (self._length_cm * self._width_cm
* self._height_cm)
return max(self._weight_kg, volume // 5000)
class ShippingQuote:
def __init__(self, rate_per_kg: int) -> None:
self._rate_per_kg = rate_per_kg
def price(self, parcel: Parcel) -> str:
cost = parcel.billable_kg() * self._rate_per_kg
return f"{parcel.name}: {cost} USD"
class LabelPrinter:
def print(self, p: Parcel) -> str:
return f"{p.name} label: {p.billable_kg()} kg"
def main() -> None:
boots = Parcel("Boots", 3, 50, 40, 30)
books = Parcel("Books", 8, 20, 20, 10)
quote = ShippingQuote(2)
print(quote.price(boots))
print(quote.price(books))
print(LabelPrinter().print(boots))Transcript, step by step
Problem
Step 1: A parcel quote Before
This shop works out the shipping price for each parcel before it goes out. A Parcel keeps a name, a weight in kilograms, and three sizes in centimeters, and the leading underscore tells readers those fields are private. It has a getter for each one, and here a getter is a read-only property that hands out a field. Shipping Quote prices a parcel at a rate per kilogram, and main is the caller. The carrier charges for billable weight, which is the larger of the real weight and a size-based weight. The size-based weight is the volume divided by five thousand, and since the shop ships with one carrier, that divisor is a fixed part of the rule. The quote is where the price is needed, so the weight math went there too, and that was a fair first choice. Boots weighs three kilos but its size counts as twelve, so it costs twenty four dollars, and Books costs sixteen for its real eight kilos.
Step 2: Billable weight on the label Change request
Now the warehouse wants the billable weight printed on every label. The quick edit adds a Label Printer, and it copies the volume line and the billable weight line from the quote. Main prints a label for the boots, so the run adds Boots label, twelve kilograms. Mypy passes, and for now the label and the price agree.
Step 3: Count the reads Code smell
Both methods do their math almost entirely on a Parcel's data, and they reach it through its getters. You can see it in Shipping Quote, which reads the three sizes here and the weight here, and Label Printer reads them again here and here. So the billable weight rule now lives twice, in two classes that own none of its data. When the carrier moves from five thousand to four thousand, both classes change, and the copy you forget prints a label that disagrees with the price. This smell is called feature envy, a method that uses another object's data more than its own.
Refactor
Step 4: Refactor first Back it out
So the label printer comes back out for now, and the run prints only the two prices again. We'll move the weight math into Parcel in small steps, and then the label becomes a single call.
Step 5: Split off the weight math Extract Method
First, in Shipping Quote, the volume line and the billable weight line become a private method called billable kg. Price calls it, and the run prints the same two prices. Now count the reads, because the new method reads four Parcel fields and never touches self. Price keeps the rate per kilogram, which is the quote's own data. That's why we extract first, since it splits the part that envies Parcel from the part that belongs in the quote.
Step 6: Parcel owns the math Move Method
Next, Move Method puts billable kg into Parcel, the class that owns every field it reads. Inside Parcel there's no getter in the math, because the class reads its own fields directly. Price now makes one call to the parcel, and the output stays the same.
Step 7: Delete the unused getters Remove Dead Code
The getters for the weight and the three sizes have no callers now, so we delete them. Name stays, because price uses it to put the parcel's name in the message.
Payoff
Step 8: The label, once Payoff
Now the label goes back in, and Label Printer is a single return statement. Main prints the boots label again, and the run adds Boots label, twelve kilograms. Parcel's billable kg and Shipping Quote did not change. The two places that read four getters each are now one call, here and here. The method reads only Parcel's own fields and one fixed constant, so Move Method put it in Parcel. With one carrier and one divisor, nothing outside Parcel pulls the method away, and the billable weight rule lives in one place.
Recap
Step 9: Where we started, where we ended Before / after
On the left, Parcel handed out every field, and the quote read four of them to work out the billable weight. Any class that needed the same number had to copy that math. On the right, Parcel works out its billable weight from its own fields. The quote and the label printer each make one call, and Parcel hands out only its name.
Step 10: When to reach for it Takeaways
Use it when a method reads several fields of one object and only a few of its own. Skip it when the divisor differs per carrier, because then the method reads the carrier's data as much as the parcel's, so pass the divisor in or give it to a carrier policy object. Skip it too for plain data crossing a boundary, like a request body or a database row, since that type is serialized and shared. You'll spot it by a run of getter reads on one parameter, and the same math wherever that object goes. Python's own timedelta has total seconds, so a caller asks it instead of adding up its days and seconds by hand.