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
declare(strict_types=1);
final class Parcel
{
public function __construct(
private readonly string $name,
private readonly int $weightKg,
private readonly int $lengthCm,
private readonly int $widthCm,
private readonly int $heightCm,
) {}
public function getName(): string
{
return $this->name;
}
public function getWeightKg(): int
{
return $this->weightKg;
}
public function getLengthCm(): int
{
return $this->lengthCm;
}
public function getWidthCm(): int
{
return $this->widthCm;
}
public function getHeightCm(): int
{
return $this->heightCm;
}
}
final class ShippingQuote
{
public function __construct(
private readonly int $ratePerKg,
) {}
public function price(Parcel $parcel): string
{
$volume = $parcel->getLengthCm() * $parcel->getWidthCm()
* $parcel->getHeightCm();
$billableKg = max($parcel->getWeightKg(), intdiv($volume, 5000));
$cost = $billableKg * $this->ratePerKg;
return $parcel->getName() . ': ' . $cost . ' USD';
}
}
$boots = new Parcel('Boots', 3, 50, 40, 30);
$books = new Parcel('Books', 8, 20, 20, 10);
$quote = new ShippingQuote(2);
echo $quote->price($boots), "\n";
echo $quote->price($books), "\n";After
declare(strict_types=1);
final class Parcel
{
public function __construct(
private readonly string $name,
private readonly int $weightKg,
private readonly int $lengthCm,
private readonly int $widthCm,
private readonly int $heightCm,
) {}
public function getName(): string
{
return $this->name;
}
public function billableKg(): int
{
$volume = $this->lengthCm * $this->widthCm
* $this->heightCm;
return max($this->weightKg, intdiv($volume, 5000));
}
}
final class ShippingQuote
{
public function __construct(
private readonly int $ratePerKg,
) {}
public function price(Parcel $parcel): string
{
$cost = $parcel->billableKg() * $this->ratePerKg;
return $parcel->getName() . ': ' . $cost . ' USD';
}
}
final class LabelPrinter
{
public function print(Parcel $p): string
{
return $p->getName() . ' label: ' . $p->billableKg() . ' kg';
}
}
$boots = new Parcel('Boots', 3, 50, 40, 30);
$books = new Parcel('Books', 8, 20, 20, 10);
$quote = new ShippingQuote(2);
echo $quote->price($boots), "\n";
echo $quote->price($books), "\n";
echo (new LabelPrinter())->print($boots), "\n";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, all private and read-only, so PHP refuses access from outside the class. 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 the script at the bottom 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. The script prints a label for the boots, so the run adds Boots label, twelve kilograms. PHPStan 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 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 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. 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. The script 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 called 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 called four getters 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 makes 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. PHP's own Date Time Immutable formats itself, so a caller asks it for a date string instead of reading the year, month and day by hand.