Feature Envy

in reviewThis lesson is complete and being validated before release - details may still change.

Feature envy

Loading interactive demo…

Fowler's smell of a method that "seems more interested in a class other than the one it is in", measured the way Lanza and Marinescu measure it: a fixture program's classes drawn as boxes of members, and a detector reading every field each method touches and crediting it to the class that declares it. Three numbers come out: access to foreign data, the distinct fields of other classes read; locality of attribute accesses, the share of the data that is the method's own; and foreign data providers, how many classes the foreign fields belong to. The refactor knob applies Move Method, the library's toy form: the method goes to the class whose data it reads, the handle it reached that class through becomes this, and what it still needs from its old class arrives as a trailing parameter; every caller follows.Take the tour, or press and explore. Legend:

The panel beside the stage is the real FeatureEnvy class from the published library executing in your browser, with the current line highlighted.

The method being tallied and the read in progress.
An own read: an edge from the method to a field of its own class.
A foreign read, and the envied class, whose rows take the method's share of its data.
A method not yet reached; an edge is dashed until its read plays.

own data and foreign data count distinct fields, locality is the own share of both together, providers the classes the foreign fields belong to, verdict the three against few and a third, member what the tapped member reads or who reads it. Tap a member to pick it.

The code panel shows bare executable code by default. The bar above it names the file (hover the name for the full published path); options holds the toggles: comments, the tracer instrumentation that produces this visualization, and verbose names for the terse identifiers (display only: the default is the byte-exact published file). The view switch adds plain-English narration: both interleaves it with the code, pseudo shows only the pseudocode. hide code collapses the panel; the rail it leaves behind brings it back.

Keys: Space play/pause · ←/→ step · Home/End jump · R reset · F fullscreen · G guided tour

The detector counts field reads by owner. Every field expression in a method's body is credited to the class that declares the field: the method's own class for this.x, and for customer.name or this.parcel.weight the class the target is typed as. Distinct fields of other classes are ATFD, access to foreign data; distinct own fields are own data; LAA, locality of attribute accesses, is the own share of both together; FDP counts the classes the foreign fields belong to. The cohesion lesson measured whether a class's methods share its fields; this one asks whether a method's fields are the class's at all.

Three numbers make one rule. A method is envious when ATFD is over FEW, LAA is under a third, and FDP is at most FEW. The first says it reads a lot of someone else's data, the second that it reads little of its own, the third that the someone else is one or two classes, so the method has a home to move to. Drop any one and the rule misfires: a method that reads six fields across six classes is a broad dependency, not envy.

Move Method follows the data. The method goes to the class whose fields it reads; the handle it reached that class through, a parameter or a field of that type, becomes this, so every foreign read turns into an own read. What it still needs from its old class arrives through a trailing parameter of that class's type. Every caller follows: invoice.header(customer) becomes customer.header(), and this.shipment.cost() becomes this.shipment.parcel.cost(this.shipment).

The thresholds sit at boundaries on purpose. The cart's total reads three line item fields, exactly FEW, and stays clean because the rule says more than FEW. The shipment's cost reads five parcel fields against two of its own, locality 2 of 7, just under a third; one foreign field fewer and it would sit at a third exactly and pass. Slide few and watch the verdict move while every count holds still.

What the stage does not show. The refactoring here is the library's toy form: exactly one handle of the target type, a free name on the target, and the old class reached by one trailing parameter. Real moves negotiate several handles, accessors, and methods that are only partly envious, where Fowler says extract the envious part first and move that.

Choices worth reading closely in the implementation (the code panel's comments and tracer checkboxes reveal the full published file):

Ownership is resolved by type, not by name. ownerOf asks the program query what the target of a field expression is typed as, through parameters, locals, loop variables, fields, and call returns; a target typed outside the program (string, LineItem[]) is no access at all, which is why this.items.length does not count as reading a line item.
Fields are visited inner before outer. eachField walks an expression post-order, so this.parcel.weight is recorded as the own read of parcel and then the foreign read of weight, and the trace shows both edges in that order.
Repeats are events, not counts. Every read emits an access, so the stage can weight an edge by how often a field is read, while the tally keeps distinct fields in sets and credits a provider once per distinct field.
Accessor calls are not reads. A getA() call is a call, so a class that hides its fields behind getters shows no envy to this detector; the real strategy treats accessors as data too, and the doc comment says this is the toy form's blind spot.
The thresholds are constructor arguments, so the few knob rebuilds the detector and only the verdict payload changes; the strict > on ATFD and < on LAA are what keep the two boundary fixtures on the clean side.

Objects were meant to keep data and the behaviour that uses it in one place, and Riel's design heuristics said so in 1996. The smell that breaks the rule got its name three years later, when Fowler and Beck catalogued a method more interested in a class other than its own, with Move Method as the cure. The numbers came after: Marinescu turned such smells into detection strategies, rules over metrics, and with Lanza published the three that measure this one: foreign data, locality, and providers. This lesson runs that rule on a small program and then on the moved method.

1996Arthur J. Riel - Object-Oriented Design Heuristics gathers sixty-one rules of thumb for class design, among them that data and the behaviour that depends on it should live in one class, the principle this smell violates.source
1999Martin Fowler and Kent Beck - Refactoring: Improving the Design of Existing Code names Feature Envy in its catalogue of smells, a method that seems more interested in a class other than the one it is in, and prescribes Move Method, with Extract Method first when only part of the method is envious.source
2004Radu Marinescu - 'Detection Strategies: Metrics-Based Rules for Detecting Design Flaws' at the International Conference on Software Maintenance proposes expressing a design flaw as a logical rule over metrics with thresholds, so a tool can find it.source
2006Michele Lanza and Radu Marinescu - Object-Oriented Metrics in Practice publishes the Feature Envy detection strategy this lesson traces: ATFD over FEW, LAA under ONE THIRD, and FDP at most FEW, with FEW set to 3.source

Used today

Move Method in the IDE - Refactoring tools move a method to the class of a parameter or field and rewrite every caller, the mechanical half of the cure.
Quality servers - Metric suites report ATFD and LAA per method and flag the ones over the thresholds for review.
Code review - A reviewer who sees a method reaching through a parameter for field after field asks why it does not live there.

scenario

refactororiginal

few3

own data: 0
foreign data: 4
locality: 0 of 4
providers: 1 class
verdict: envies Customer
member: subtotal: total

feature-envy.ts

import type { ClassDecl, Expr, MemberRef, Method, Program } from "./program";import type { Tracer } from "../trace";import type { SmellDetector } from "./detector";import { ProgramQuery } from "./program-query";export interface EnvyThresholds {  /** Lanza and Marinescu's FEW: more foreign fields than this is envy, and more providers than this is not envy but a broad dependency. */  readonly few: number;  /** Their ONE THIRD: a method whose own share of the data it touches is under this is envious. */  readonly ownShare: number;}export const DEFAULT_ENVY_THRESHOLDS: EnvyThresholds = { few: 3, ownShare: 1 / 3 };export interface DataAccess {  /** The statement the field expression sits in. */  readonly nodeId: string;  /** The class that declares the field: the method's own class, or the class the target types to. */  readonly owner: string;  readonly field: string;  /** Whether the owner is another class than the method's. */  readonly foreign: boolean;}export interface EnvyReport {  readonly method: string;  readonly className: string;  /** Every field access in statement order, inner before outer, repeats included. */  readonly accesses: readonly DataAccess[];  /** ATFD: distinct foreign fields, counted as `Owner.field`. */  readonly atfd: number;  /** Distinct own fields. */  readonly ownData: number;  /** LAA: own / (own + foreign); 1 when the method touches no data. */  readonly laa: number;  /** FDP: distinct classes the foreign fields belong to. */  readonly fdp: number;  /** Each foreign class with how many of its fields the method reads. */  readonly providers: Readonly<Record<string, number>>;  /** The provider with the most fields read; null when there is none. */  readonly envied: string | null;  readonly smelly: boolean;}interface Tally {  readonly accesses: DataAccess[];  readonly own: Set<string>;  readonly foreign: Set<string>;  readonly providers: Record<string, number>;}/** * Feature envy, Fowler's smell of a method that is more interested in * another class's data than in its own, measured with the detection * strategy of Lanza and Marinescu: ATFD (access to foreign data) counts * the distinct fields of other classes the method reaches, LAA (locality * of attribute accesses) is the share of the touched fields that are the * method's own, and FDP (foreign data providers) counts the classes * those foreign fields belong to. The method is envious when ATFD is * over FEW, LAA is under ONE THIRD, and FDP is at most FEW: it reads a * lot of someone else's data, little of its own, and that someone is * one or two classes, so the method has a home to move to. * * A data access is a `field` expression whose target is `this`, or * whose target types (through `ProgramQuery.typeOf`) to a class of the * program: a parameter, a local, a loop variable, a field, or a call * returning that class. A target of a type outside the program (`string`, * `Item[]`) is ignored, so `this.items.length` is not an access to * `Item`. Accessor calls are calls, not field expressions, and are not * counted; this toy form sees data only when it is named as a field. * * Time: O(n · d) over n expression nodes and the depth d a target's * type is resolved through. Space: O(n). */export class FeatureEnvy implements SmellDetector<MemberRef, EnvyReport> {  constructor(    readonly program: Program,    readonly thresholds: EnvyThresholds = DEFAULT_ENVY_THRESHOLDS,    private readonly tracer?: Tracer,  ) {}  analyze(target: MemberRef): EnvyReport {    const query = new ProgramQuery(this.program);    const cls = query.class(target.className);    const method = query.method(target);    return this.analyzeMethod(query, cls, method, cls.methods.indexOf(method), cls.methods.length);  }  /** Every method of the class, in declaration order, as one trace. */  analyzeClass(cls: ClassDecl): readonly EnvyReport[] {    const query = new ProgramQuery(this.program);    return cls.methods.map((method, index) => this.analyzeMethod(query, cls, method, index, cls.methods.length));  }  private analyzeMethod(query: ProgramQuery, cls: ClassDecl, method: Method, index: number, count: number): EnvyReport {    // ▸ a method: its tally of own and foreign data opens    this.tracer?.emit("method", { className: cls.name, count, index, name: method.name });    const locals = ProgramQuery.locals(method);    const tally: Tally = { accesses: [], own: new Set(), foreign: new Set(), providers: {} };    ProgramQuery.walk(method.body, (stmt) => {      for (const root of ProgramQuery.exprs(stmt)) {        this.eachField(root, (expr) => {          const owner = this.ownerOf(query, expr, cls, locals);          if (owner !== null) this.record(tally, stmt.id, owner, expr.name, cls, method);        });      }    });    const atfd = tally.foreign.size;    const ownData = tally.own.size;    const laa = atfd + ownData === 0 ? 1 : ownData / (ownData + atfd);    const fdp = Object.keys(tally.providers).length;    // ▸ own fields, foreign fields, and the classes they belong to    this.tracer?.emit("tally", { atfd, fdp, laa, method: method.name, ownData, providers: { ...tally.providers } });    let envied: string | null = null;    for (const [owner, fields] of Object.entries(tally.providers)) {      if (envied === null || fields > tally.providers[envied]!) envied = owner;    }    const { few, ownShare } = this.thresholds;    const smelly = atfd > few && laa < ownShare && fdp <= few;    // ▸ the three numbers against few and a third    this.tracer?.emit("verdict", { atfd, className: cls.name, envied, fdp, few, laa, method: method.name, ownShare, smelly });    return { method: method.name, className: cls.name, accesses: tally.accesses, atfd, ownData, laa, fdp, providers: tally.providers, envied, smelly };  }  /** One access: own fields by name, foreign fields by `Owner.field`, each distinct one credited to its provider. */  private record(tally: Tally, nodeId: string, owner: string, field: string, cls: ClassDecl, method: Method): void {    const foreign = owner !== cls.name;    if (foreign) {      const key = `${owner}.${field}`;      if (!tally.foreign.has(key)) {        tally.foreign.add(key);        tally.providers[owner] = (tally.providers[owner] ?? 0) + 1;      }    } else {      tally.own.add(field);    }    tally.accesses.push({ nodeId, owner, field, foreign });    // ▸ one field read, credited to the class that declares it    this.tracer?.emit("access", { atfd: tally.foreign.size, field, foreign, method: method.name, nodeId, owner, ownData: tally.own.size });  }  /** The class that owns the field expression's data: the method's own class for `this.x`, else the target's type when it is a class of the program, else null. */  private ownerOf(query: ProgramQuery, expr: Expr & { kind: "field" }, cls: ClassDecl, locals: ReadonlyMap<string, string>): string | null {    if (expr.target.kind === "this") return cls.name;    const type = query.typeOf(expr.target, cls, locals);    return type !== null && query.classOrNull(type) !== null ? type : null;  }  /** Post-order over an expression tree, so `this.parcel` is seen before `this.parcel.weight`. */  private eachField(expr: Expr, visit: (field: Expr & { kind: "field" }) => void): void {    switch (expr.kind) {      case "field":        this.eachField(expr.target, visit);        visit(expr);        break;      case "call":        this.eachField(expr.target, visit);        for (const arg of expr.args) this.eachField(arg, visit);        break;      case "new":        for (const arg of expr.args) this.eachField(arg, visit);        break;      case "unary":        this.eachField(expr.operand, visit);        break;      case "binary":        this.eachField(expr.left, visit);        this.eachField(expr.right, visit);        break;      default:        break;    }  }}

method =: header
atfd =: 4
ownData =: 0
laa =: 0
fdp =: 1
envied =: Customer

Invoice.header: 4 foreign, 0 own, locality 0%. Still envies Customer.

import type { ClassDecl, Expr, MemberRef, Method, Program } from "./program";import type { Tracer } from "../trace";import type { SmellDetector } from "./detector";import { ProgramQuery } from "./program-query";export interface EnvyThresholds { /** Lanza and Marinescu's FEW: more foreign fields than this is envy, and more providers than this is not envy but a broad dependency. */ readonly few: number; /** Their ONE THIRD: a method whose own share of the data it touches is under this is envious. */ readonly ownShare: number;}export const DEFAULT_ENVY_THRESHOLDS: EnvyThresholds = { few: 3, ownShare: 1 / 3 };export interface DataAccess { /** The statement the field expression sits in. */ readonly nodeId: string; /** The class that declares the field: the method's own class, or the class the target types to. */ readonly owner: string; readonly field: string; /** Whether the owner is another class than the method's. */ readonly foreign: boolean;}export interface EnvyReport { readonly method: string; readonly className: string; /** Every field access in statement order, inner before outer, repeats included. */ readonly accesses: readonly DataAccess[]; /** ATFD: distinct foreign fields, counted as `Owner.field`. */ readonly atfd: number; /** Distinct own fields. */ readonly ownData: number; /** LAA: own / (own + foreign); 1 when the method touches no data. */ readonly laa: number; /** FDP: distinct classes the foreign fields belong to. */ readonly fdp: number; /** Each foreign class with how many of its fields the method reads. */ readonly providers: Readonly<Record<string, number>>; /** The provider with the most fields read; null when there is none. */ readonly envied: string | null; readonly smelly: boolean;}interface Tally { readonly accesses: DataAccess[]; readonly own: Set<string>; readonly foreign: Set<string>; readonly providers: Record<string, number>;}/** * Feature envy, Fowler's smell of a method that is more interested in * another class's data than in its own, measured with the detection * strategy of Lanza and Marinescu: ATFD (access to foreign data) counts * the distinct fields of other classes the method reaches, LAA (locality * of attribute accesses) is the share of the touched fields that are the * method's own, and FDP (foreign data providers) counts the classes * those foreign fields belong to. The method is envious when ATFD is * over FEW, LAA is under ONE THIRD, and FDP is at most FEW: it reads a * lot of someone else's data, little of its own, and that someone is * one or two classes, so the method has a home to move to. * * A data access is a `field` expression whose target is `this`, or * whose target types (through `ProgramQuery.typeOf`) to a class of the * program: a parameter, a local, a loop variable, a field, or a call * returning that class. A target of a type outside the program (`string`, * `Item[]`) is ignored, so `this.items.length` is not an access to * `Item`. Accessor calls are calls, not field expressions, and are not * counted; this toy form sees data only when it is named as a field. * * Time: O(n · d) over n expression nodes and the depth d a target's * type is resolved through. Space: O(n). */export class FeatureEnvy implements SmellDetector<MemberRef, EnvyReport> { constructor( readonly program: Program, readonly thresholds: EnvyThresholds = DEFAULT_ENVY_THRESHOLDS, private readonly tracer?: Tracer, ) {} analyze(target: MemberRef): EnvyReport { const query = new ProgramQuery(this.program); const cls = query.class(target.className); const method = query.method(target); return this.analyzeMethod(query, cls, method, cls.methods.indexOf(method), cls.methods.length); } /** Every method of the class, in declaration order, as one trace. */ analyzeClass(cls: ClassDecl): readonly EnvyReport[] { const query = new ProgramQuery(this.program); return cls.methods.map((method, index) => this.analyzeMethod(query, cls, method, index, cls.methods.length)); } private analyzeMethod(query: ProgramQuery, cls: ClassDecl, method: Method, index: number, count: number): EnvyReport { // ▸ a method: its tally of own and foreign data opens this.tracer?.emit("method", { className: cls.name, count, index, name: method.name }); const locals = ProgramQuery.locals(method); const tally: Tally = { accesses: [], own: new Set(), foreign: new Set(), providers: {} }; ProgramQuery.walk(method.body, (stmt) => { for (const root of ProgramQuery.exprs(stmt)) { this.eachField(root, (expr) => { const owner = this.ownerOf(query, expr, cls, locals); if (owner !== null) this.record(tally, stmt.id, owner, expr.name, cls, method); }); } }); const atfd = tally.foreign.size; const ownData = tally.own.size; const laa = atfd + ownData === 0 ? 1 : ownData / (ownData + atfd); const fdp = Object.keys(tally.providers).length; // ▸ own fields, foreign fields, and the classes they belong to this.tracer?.emit("tally", { atfd, fdp, laa, method: method.name, ownData, providers: { ...tally.providers } }); let envied: string | null = null; for (const [owner, fields] of Object.entries(tally.providers)) { if (envied === null || fields > tally.providers[envied]!) envied = owner; } const { few, ownShare } = this.thresholds; const smelly = atfd > few && laa < ownShare && fdp <= few; // ▸ the three numbers against few and a third this.tracer?.emit("verdict", { atfd, className: cls.name, envied, fdp, few, laa, method: method.name, ownShare, smelly }); return { method: method.name, className: cls.name, accesses: tally.accesses, atfd, ownData, laa, fdp, providers: tally.providers, envied, smelly }; } /** One access: own fields by name, foreign fields by `Owner.field`, each distinct one credited to its provider. */ private record(tally: Tally, nodeId: string, owner: string, field: string, cls: ClassDecl, method: Method): void { const foreign = owner !== cls.name; if (foreign) { const key = `${owner}.${field}`; if (!tally.foreign.has(key)) { tally.foreign.add(key); tally.providers[owner] = (tally.providers[owner] ?? 0) + 1; } } else { tally.own.add(field); } tally.accesses.push({ nodeId, owner, field, foreign }); // ▸ one field read, credited to the class that declares it this.tracer?.emit("access", { atfd: tally.foreign.size, field, foreign, method: method.name, nodeId, owner, ownData: tally.own.size }); } /** The class that owns the field expression's data: the method's own class for `this.x`, else the target's type when it is a class of the program, else null. */ private ownerOf(query: ProgramQuery, expr: Expr & { kind: "field" }, cls: ClassDecl, locals: ReadonlyMap<string, string>): string | null { if (expr.target.kind === "this") return cls.name; const type = query.typeOf(expr.target, cls, locals); return type !== null && query.classOrNull(type) !== null ? type : null; } /** Post-order over an expression tree, so `this.parcel` is seen before `this.parcel.weight`. */ private eachField(expr: Expr, visit: (field: Expr & { kind: "field" }) => void): void { switch (expr.kind) { case "field": this.eachField(expr.target, visit); visit(expr); break; case "call": this.eachField(expr.target, visit); for (const arg of expr.args) this.eachField(arg, visit); break; case "new": for (const arg of expr.args) this.eachField(arg, visit); break; case "unary": this.eachField(expr.operand, visit); break; case "binary": this.eachField(expr.left, visit); this.eachField(expr.right, visit); break; default: break; } }}