# Feature Envy

> A method reads and computes on another object's data more than its own. Move the logic to where the data lives.

- kind: smell
- category: couplers
- canonical: https://aipatterns.dev/refactoring/smells/feature-envy
- inspired-by: https://refactoring.guru/smells/feature-envy

## Signs

- A method calls several getters on one other object and few or none on `this`.
- The method's result depends almost entirely on another type's fields.
- You see the same chain of reads on that other object in several places.
- The method's name mentions the other type: `computeInvoiceTotal` living in `ReportService`.
- Changing the other type's fields forces edits in this method, never the reverse.

## Why it happens

- Data and behavior started in different places. Someone made a plain record type and wrote the logic in a service next to the code that first needed it.
- Moving a field to a new class left behind the methods that used it.
- A layered architecture rule ("models hold data, services hold logic") was applied mechanically, so every calculation sits away from its inputs.
- Each new caller copied the calculation instead of asking the data owner for it.

## Example

A shipping service computes everything about a parcel from the parcel's own fields.

```ts title="before.ts"
class Parcel {
  constructor(
    public lengthCm: number,
    public widthCm: number,
    public heightCm: number,
    public weightKg: number,
  ) {}
}

class ShippingService {
  billableWeight(p: Parcel): number {
    const volumetric = (p.lengthCm * p.widthCm * p.heightCm) / 5000;
    return Math.max(p.weightKg, volumetric);
  }

  isOversized(p: Parcel): boolean {
    return Math.max(p.lengthCm, p.widthCm, p.heightCm) > 120;
  }

  quote(p: Parcel, ratePerKg: number): number {
    return Math.ceil(this.billableWeight(p)) * ratePerKg;
  }
}
```

`billableWeight` and `isOversized` touch only `Parcel`. They belong there.

```ts title="after.ts"
class Parcel {
  constructor(
    readonly lengthCm: number,
    readonly widthCm: number,
    readonly heightCm: number,
    readonly weightKg: number,
  ) {}

  billableWeight(): number {
    const volumetric = (this.lengthCm * this.widthCm * this.heightCm) / 5000;
    return Math.max(this.weightKg, volumetric);
  }

  isOversized(): boolean {
    return Math.max(this.lengthCm, this.widthCm, this.heightCm) > 120;
  }
}

class ShippingService {
  quote(p: Parcel, ratePerKg: number): number {
    return Math.ceil(p.billableWeight()) * ratePerKg;
  }
}
```

`quote` stays in the service because the rate belongs to the service, not the parcel.

## Treatment

- If the whole method works on the other object's data, use [Move Method](https://aipatterns.dev/refactoring/techniques/move-method.md).
- If only part of the method envies the other object, first use [Extract Method](https://aipatterns.dev/refactoring/techniques/extract-method.md) on that part, then move the extracted piece.
- If the method uses data from several objects, move it to the object whose data it uses most. Pass the rest in as arguments.
- If a field is read mostly by another class, consider [Move Field](https://aipatterns.dev/refactoring/techniques/move-field.md) instead of moving the methods.

## Payoff

- Behavior sits next to the data it depends on, so a change to that data touches one class.
- Fewer getters leak internal fields; some can become private.
- Duplicate calculations across callers collapse into one method.

## When to ignore

- The method deliberately sits apart from the data: [Strategy](https://aipatterns.dev/design-patterns/strategy.md) and [Visitor](https://aipatterns.dev/design-patterns/visitor.md) put behavior outside the data so it can vary independently.
- The data type is a DTO or wire format shared across services, and adding logic would couple consumers to it.
- The other type is generated code or a third-party class you cannot edit. See [Incomplete Library Class](https://aipatterns.dev/refactoring/smells/incomplete-library-class.md).
- In a functional codebase, plain data plus free functions is the convention. Keep the functions in the module that owns the type rather than moving them into the type.

## Agent checklist

- [ ] Count reads on `this` versus reads on the other object before moving anything.
- [ ] Confirm the target class is editable and is not a shared DTO or generated type.
- [ ] Extract the envious fragment first if only part of the method qualifies.
- [ ] Leave a delegating method in the old place if public callers depend on it, and mark it for removal.
- [ ] Run the tests after the move and check no new circular import appeared.