Feature Envy
A function that reaches into another module's data far more than its own. Sometimes the behaviour is in the wrong place; sometimes the other module is a value type and this is exactly right.
The requirement, the obvious build, and why it breaks
Every lesson starts where the work starts: someone asked for something, and the first implementation that comes to mind survives until the requirement changes.
This method uses six fields of another object and none of its own. Does the behaviour belong over there?
Add loyalty-tier discounts. The pricing code already reaches into the customer record for country, signup date, order history and account status; now it needs tier and points too.
Pricing needs those fields, so pricing reads them. Getters exist for exactly this; the data is public in all but name and everything works.
Every new attribute the rule needs widens the coupling: pricing now knows the customer's field names, their types, their nullability and their meaning. That is not one dependency, it is one per field.
- Every new attribute the rule needs widens the coupling: pricing now knows the customer's field names, their types, their nullability and their meaning. That is not one dependency, it is one per field.
- The customer's internal shape is now frozen by a module that is not the customer. Renaming a column, changing an enum or making a field optional breaks pricing, which is a long way away and owned by someone else (Exposing Too Much).
- The rule is invisible from the customer's side. Someone reading
Customercannot tell that itsstatusfield participates in pricing, so a change to what a status means silently changes prices. - Once two modules reach in this way, the "same" rule appears in both, and you are back to duplicated knowledge with extra steps (Duplicate Knowledge).
What limits the solution, and what must never stop being true
This domain leads with these two. A design that ignores its constraints is not a design, and an invariant nobody named is one nothing is protecting.
- Customer is also read by three other subsystems, so moving behaviour into it makes those subsystems depend on pricing types.
- The customer record is persisted by an ORM and its shape is coupled to the table (Schema Leakage).
- The team has agreed not to introduce a new module without deleting one.
- Two customers with identical relevant attributes must receive identical prices.
- Whatever computes the discount must be reachable from exactly one place, so a change to tiers cannot half-apply.
Who owns what, and where the seams fall
Responsibilities decide boundaries; boundaries decide what an interface has to say.
- The customer module owns what its data means — what "active" is, what tier a points balance implies, how signup date maps to tenure.
- The pricing module owns what those meanings are worth in money. That is a different responsibility with a different rate of change.
- Neither owns the other's field names. The seam is a small domain vocabulary — tier, tenure band, standing — that both can speak (Ubiquitous Language).
- Move the *interpretation* of the data to the data's owner, and keep the *policy* where the policy changes.
customer.tier()belongs to the customer;discountFor(tier)belongs to pricing. - If moving the behaviour would drag pricing types into the customer module, the behaviour is not what should move — the vocabulary is (Dependency Direction).
- A value type is not a boundary you should protect this way. Reaching into a
Moneyor aDateRangeis not envy; those exist to be used (Value Objects).
Counting, not feeling
The useful form of this smell is arithmetic: in this function, how many members of another type are touched, versus how many of its own? A single number turns "this feels wrong" into something a reviewer can check without sharing your taste.
The fine case below is not a corner. A very large fraction of the code flagged by this smell in real codebases is correct, and the distinguishing question is whether the envied type owns any invariants at all.
looks like A method that reads five or six members of another object and barely touches its own. Long chains of accessors — order.customer.address.country — and conditional logic written in terms of another module's field values rather than its concepts.
suggests The meaning of that data is being reconstructed outside its owner. The reaching module now depends on the envied type's internal shape, and the same reconstruction is probably happening somewhere else too.
fix Move the interpretation to the owner and keep the policy where it is: customer.tier() on one side, discountFor(tier) on the other. Where the rule truly spans both, give it a third home that depends on both and is imported by neither.
Money, DateRange, Coordinates, a DTO deserialised from someone else's API. These exist to be read, and pushing behaviour into them would either give them dependencies they should not have or turn a serialisable value into a service. It is also fine when the reaching function *is* the designated interpreter: a single PricingInput.from(customer) mapper that concentrates the reaching in one place is a legitimate answer, and often better than scattering derived accessors across the envied type (Boundary Adapters).1// Envy: pricing reconstructs what a customer IS.2if (c.status === 'A' && c.points > 5000 &&3 daysSince(c.signupAt) > 730) discount = 0.154 5// Moved too far: customer now knows about money.6class Customer { discount(): number { ... } } // changes with7 // every promo8 9// Split by rate of change:10class Customer { tier(): Tier; tenure(): TenureBand }11const discount = pricing.discountFor(c.tier(), c.tenure())The third version is the only one where a change to what "gold" means and a change to what gold is worth land in different files. That is the whole point; the reduction in accessor chains is incidental.
Three placements, priced
Placement decisions look like style arguments and are not. Score them across the axes that actually move and the disagreement usually turns out to be about which axis the team is currently paying for.
| Option | Simplicity | Flexibility | Testability | Migration cost | Note |
|---|---|---|---|---|---|
| Leave the reaching code in pricing | Simplest today and no migration at all. Costs a dependency on the customer's field shape, and the interpretation will be duplicated by the next subsystem that needs it. | ||||
| Move the whole rule onto Customer | One place, easy to find — and the customer module now changes whenever marketing changes a promotion, and imports pricing types. Worst rate-of-change pairing of the three. | ||||
| Vocabulary on Customer, policy in pricing | Two small changes instead of one, and the two rates of change are separated. Testable without a customer record on one side and without a clock on the other. | ||||
| A dedicated mapper at the boundary | Concentrates all reaching in one adapter. Good when the envied type is external or ORM-shaped and cannot be given behaviour (Anti-Corruption Layer). |
caveat These numbers compare placements for *this* rule, in a codebase where customer data is stable and pricing changes monthly. Reverse those rates — a volatile customer model and a fixed price list — and the second and third rows swap. Nothing in the scores expresses the thing that actually decides: which of the two modules you expect to be edited more often, which is a product question rather than a design one.
When to leave it alone
Reflexive fixing of this smell is how codebases acquire entities with forty methods, half of which exist to serve one caller. The checklist below is deliberately biased towards inaction, because inaction is right more often here than for any other smell in the module.
| Trigger | Symptom | Cause | Response |
|---|---|---|---|
| Envied type is a value object | Money now imports the tax module to offer withVat() | A value with no invariants was treated as a missing owner of behaviour | Revert. Values are for reading; the behaviour belongs to whoever owns the rule (Value Objects). |
| Envied type is an external DTO | Generated client classes hand-edited, and lost at the next regeneration | Behaviour pushed onto a type you do not own | Map it at the boundary into your own type and put behaviour there (Anti-Corruption Layer). |
| One caller, one field, once | A derived accessor with exactly one call site and its own test | A rule applied by count rather than by recurrence | Inline it back. One caller is not a pattern (The Rule of Three). |
| Behaviour moved to satisfy a linter | Entity grows a method per consumer; the entity is now in every diff | Placement decided by tooling rather than by rate of change | Group derived accessors by concept, and push consumer-specific shapes into mappers (Divergent Change). |
| Reaching is broad but read-only and stable | A report touches twelve fields and has not changed in two years | The smell is real and the code is not costing anything | Leave it. Stability is evidence that the coupling is not being exercised (When Design Does Not Pay). |
How to build it
Most important first.
- Count what the function touches on each side. Six fields of theirs and none of yours is evidence; two of theirs and eight of yours is nothing.
- Ask which module would have to change if the *meaning* changed. If the answer is the other one, the interpretation belongs there.
- Prefer moving a small derivation over moving the whole rule.
customer.tenureBand()is cheap, testable and does not import pricing (Extract Function). - Where the rule genuinely spans both, put it in a third place that depends on both and is depended on by neither — a domain service, not a helper (Domain Services).
- Leave it alone when the "envied" object is a plain value or a DTO from outside your system. Anti-corruption happens at the edge, not by giving the DTO behaviour (Anti-Corruption Layer).
What the next change costs
The field this whole domain exists for. A structure is only better if it makes the change after this one cheaper — and it is worth saying which changes it does not help.
- Before: renaming a customer field costs an edit in pricing, in reporting and in export, plus a search for string-keyed access the compiler will not find. Adding a nullable field costs a null check in every reaching module, discovered at runtime in whichever one you forgot.
- Before: changing what a tier *means* costs a change everywhere the meaning was reconstructed from raw fields, which is the expensive part and does not show up as a compile error.
- After: renaming a field costs one edit inside the customer module. Changing what a tier means costs one edit in the same place. Changing what a tier is *worth* costs one edit in pricing, and the two changes never collide.
- What is now more expensive: a genuinely new derived attribute needs a change in two modules and a conversation between two owners, where before one person could add a getter call and ship.
- Domain vocabulary on the data owner is more code than a getter, and every derived accessor is a small commitment the owner now maintains.
- Pushing behaviour towards data is exactly the move that produces god objects when applied without limit; the limit is that the owner must not acquire the other module's reasons to change.
- For anemic records that mirror database rows, adding behaviour fights the ORM and the team's habits, and half-doing it is worse than not starting (The Anemic Domain Model).
What can go wrong
- The behaviour is moved into the data object, which now imports three subsystems and becomes the god object (God Object).
- The behaviour is moved into a "helper" that both import, which is the dumping ground in miniature (The Utility Dumping Ground).
- Getters are replaced by a wide
asPricingInput()method, which is the same coupling with a nicer name and one more thing to keep in sync. - The mitigation fails: adding
customer.tier()looks harmless until four subsystems each want their own derived accessor, and the customer module accretes them all.
- Reaching in creates a dependency on structure; asking creates a dependency on vocabulary. The second is narrower and survives refactoring of the first.
- Moving behaviour the wrong way inverts the dependency into something worse: a customer module that imports pricing is a customer module that changes when prices change.
- "Behaviour should always live with data." That is a heuristic, not a law, and applied without limit it produces the god object. Policy that changes at a different rate than the data belongs elsewhere (Composition Over Inheritance).
- "Getters are the problem." Getters are a symptom at most. A type with no getters and a wide
toMap()has exactly the same coupling (Information Hiding). - "This means our model is anemic." Not necessarily. A transaction-script system can be entirely appropriate; envy is only a finding if the reaching is broad, recurring, and about meaning rather than access (Transaction Script).
- "Move the whole rule into the entity." Pricing policy inside a customer entity means the customer changes every time marketing runs a promotion, which is the worst rate-of-change pairing available (Divergent Change).
- feature-envy
- primitive-obsession
- god-object
Testing it, and how it ages
- Test the derivation where it lives —
tenureBandagainst a clock, with no pricing involved (Time as a Dependency). - Test the policy with the vocabulary as input: given tier gold and tenure over two years, the discount is X. No customer record required, which is the sign the split worked.
- One integration test that a real customer produces the expected price, because the mapping between the two is now a seam and seams are where the bugs go.
- Envy grows one field at a time and each addition is trivially defensible, so it is almost never caught in review; it is caught when someone tries to change the envied module.
- It resolves naturally when the envied type acquires a real domain vocabulary — at that point the reaching code has something better to call and usually migrates on its own.
- It comes back whenever a new subsystem arrives and finds the record easier to read than the vocabulary to learn, which is why the vocabulary has to be small enough to learn.
Where this applies
This domain's advice is contested more than most. These labels say what each claim is specific to — and where CONTESTED appears, the note gives the strongest form of the opposing view rather than a caricature.
- PARADIGM-SPECIFICThe smell is stated in OO terms — behaviour belongs with the data it uses. In a functional codebase, functions over plain data are the normal case and "envy" is not a finding at all; the equivalent question there is whether the *interpretation* of a record is implemented once or reimplemented per caller, which is a duplicate-knowledge question rather than a placement one.
- CONTESTEDThe strongest opposing view holds that separating data from the functions over it is a feature, not a defect: it keeps records serialisable, keeps modules acyclic, and lets new behaviour be added without touching existing types — the open-closed argument turned against the smell. Codebases in Go, Clojure and modern data-oriented C++ are built on this and are not worse for it. The honest scope of feature envy is OO codebases where the envied type already owns invariants.
- LANGUAGE-SPECIFICLanguages with extension methods or traits — C#, Kotlin, Rust — let you attach behaviour to a type from outside it, which dissolves the placement dilemma but reintroduces it as a question of which module owns the extension and whether two of them can disagree.
Where the depth lives
This domain teaches the codebase-level structure and hands the rest off.
- — Programming Languages & Runtime Internals — whether behaviour can be attached to a type from outside it (extension methods, traits, protocols) is a language-design question that changes this smell's answer entirely.