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(string name, int weightKg,
int lengthCm, int widthCm, int heightCm)
{
public string Name { get; } = name;
public int WeightKg { get; } = weightKg;
public int LengthCm { get; } = lengthCm;
public int WidthCm { get; } = widthCm;
public int HeightCm { get; } = heightCm;
}
class ShippingQuote(int ratePerKg)
{
public string Price(Parcel parcel)
{
int volume = parcel.LengthCm * parcel.WidthCm * parcel.HeightCm;
int billableKg = Math.Max(parcel.WeightKg, volume / 5000);
return $"{parcel.Name}: {billableKg * ratePerKg} USD";
}
}
static class Shop
{
public static void Main()
{
var boots = new Parcel("Boots", 3, 50, 40, 30);
var books = new Parcel("Books", 8, 20, 20, 10);
var quote = new ShippingQuote(2);
Console.WriteLine(quote.Price(boots));
Console.WriteLine(quote.Price(books));
}
}After
class Parcel(string name, int weightKg,
int lengthCm, int widthCm, int heightCm)
{
public string Name { get; } = name;
private readonly int weightKg = weightKg;
private readonly int lengthCm = lengthCm;
private readonly int widthCm = widthCm;
private readonly int heightCm = heightCm;
public int BillableKg()
{
int volume = lengthCm * widthCm * heightCm;
return Math.Max(weightKg, volume / 5000);
}
}
class ShippingQuote(int ratePerKg)
{
public string Price(Parcel parcel)
{
return $"{parcel.Name}: {parcel.BillableKg() * ratePerKg} USD";
}
}
class LabelPrinter
{
public string Print(Parcel p)
{
return $"{p.Name} label: {p.BillableKg()} kg";
}
}
static class Shop
{
public static void Main()
{
var boots = new Parcel("Boots", 3, 50, 40, 30);
var books = new Parcel("Books", 8, 20, 20, 10);
var quote = new ShippingQuote(2);
Console.WriteLine(quote.Price(boots));
Console.WriteLine(quote.Price(books));
Console.WriteLine(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. Each one is a get-only property, so it works as a getter, a member that hands out a value and nothing else. Shipping Quote prices a parcel at a rate per kilogram, and 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 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. Shop prints a label for the boots, so the run 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 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 properties and nothing that belongs to Shipping Quote. It never touches the rate, so your IDE would even offer to make it static. 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 value it reads. Inside Parcel the math reads its own data, with no parcel dot in front of anything. Price now makes one call to the parcel, and the output stays the same.
Step 7: Close the unused getters Remove Dead Code
No code outside Parcel reads the weight or the three sizes now, so their public getters are dead code. We delete them and keep the values in private read-only fields, which only billable kg reads. The Name property stays public, 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. Shop 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 value, 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 property reads on one parameter, and the same math wherever that object goes. In dot net, a Time Span works out its own total hours, so no caller divides its ticks.