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 {
constructor(
private readonly name: string,
private readonly weightKg: number,
private readonly lengthCm: number,
private readonly widthCm: number,
private readonly heightCm: number,
) {}
getName(): string { return this.name; }
getWeightKg(): number { return this.weightKg; }
getLengthCm(): number { return this.lengthCm; }
getWidthCm(): number { return this.widthCm; }
getHeightCm(): number { return this.heightCm; }
}
class ShippingQuote {
constructor(private readonly ratePerKg: number) {}
price(parcel: Parcel): string {
const volume = parcel.getLengthCm() *
parcel.getWidthCm() * parcel.getHeightCm();
const sizeKg = Math.floor(volume / 5000);
const billableKg =
Math.max(parcel.getWeightKg(), sizeKg);
const cost = billableKg * this.ratePerKg;
return `${parcel.getName()}: ${cost} USD`;
}
}
function runShop(): string[] {
const boots = new Parcel("Boots", 3, 50, 40, 30);
const books = new Parcel("Books", 8, 20, 20, 10);
const quote = new ShippingQuote(2);
return [
quote.price(boots),
quote.price(books),
];
}After
class Parcel {
constructor(
private readonly name: string,
private readonly weightKg: number,
private readonly lengthCm: number,
private readonly widthCm: number,
private readonly heightCm: number,
) {}
getName(): string { return this.name; }
billableKg(): number {
const volume =
this.lengthCm * this.widthCm * this.heightCm;
const sizeKg = Math.floor(volume / 5000);
return Math.max(this.weightKg, sizeKg);
}
}
class ShippingQuote {
constructor(private readonly ratePerKg: number) {}
price(parcel: Parcel): string {
const cost = parcel.billableKg() * this.ratePerKg;
return `${parcel.getName()}: ${cost} USD`;
}
}
class LabelPrinter {
print(p: Parcel): string {
return `${p.getName()} label: ${p.billableKg()} kg`;
}
}
function runShop(): string[] {
const boots = new Parcel("Boots", 3, 50, 40, 30);
const books = new Parcel("Books", 8, 20, 20, 10);
const quote = new ShippingQuote(2);
return [
quote.price(boots),
quote.price(books),
new 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 holds a name, a weight in kilograms, and three sizes in centimeters. It has a getter for each one, and a getter is a method that hands out a field. Shipping Quote prices a parcel at a rate per kilogram, and run Shop 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 rounded down, 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 both weight lines from the quote. Run Shop asks for a label for the boots, so the result adds Boots label, twelve kilograms. It compiles, 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 run Shop returns 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 three weight lines become a private method called billable kg. Price calls it, and run Shop returns the same two prices. Now count the reads, because the new method reads four Parcel fields and nothing that belongs to Shipping Quote. 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 result 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. Get 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. Run Shop asks for the boots label again, and the result 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 calls on one parameter, and the same math wherever that object goes. A Date in TypeScript formats itself, so a caller asks for the ISO string instead of reading the year, month and day one by one.