skip to content

How would you detect low cohesion in an existing class objectively, and what refactoring sequence would you apply to fix it without breaking clients?

level: seniorimportance: should knowfreq 40%

answer

  1. LCOM = *lack* of cohesion — higher is worse
  2. LCOM4 = connected components of the method/field graph; >1 → split here
  3. Version control: change frequency, author count, unrelated tickets
  4. Divergent Change (low cohesion) vs Shotgun Surgery (over-split)
  5. Extract Class → old class delegates → migrate callers → delete delegation

basics

~20 s

Look for method groups touching disjoint fields (the LCOM metric captures this), vague names, and files changed by many unrelated commits. Fix by extracting each concern into its own class, keeping the old class as a delegating facade until callers migrate.

solid answer

~50 s

**Detect** with three independent signals. *Structural*: LCOM (Lack of Cohesion of Methods) — variants like LCOM4 build a graph of methods linked when they share a field or call each other, and count connected components; more than one component means the class splits cleanly. *Historical*: version-control evidence — a file with a high change frequency, many distinct authors, and commits belonging to unrelated features/tickets has multiple reasons to change. *Human*: it cannot be named without 'and', its tests need unrelated fixtures, its imports span unrelated technical areas. **Refactor** incrementally: identify the field/method clusters; move each cluster's fields and methods into a new class (Extract Class), possibly via Extract Delegate; make the original class hold the new one and delegate, so all existing callers keep compiling; migrate callers to the new class one at a time; finally remove the dead delegation. Characterization tests first if coverage is thin. Beware: metrics are heuristics — constructors, getters, framework callbacks and dependency-injected fields distort LCOM badly.

code

pseudocode · 19 lines
pseudocode
// Field-usage table reveals two disjoint clusters (LCOM4 = 2)
class CustomerAccount {
  name, email                 // cluster A
  balance, currency           // cluster B

  updateEmail()   -> email
  displayName()   -> name, email
  charge(amount)  -> balance, currency
  convertTo(cur)  -> balance, currency
}

// Step 1: Extract Class, keep a delegating facade so callers still compile
class Money   { balance, currency; charge(a); convertTo(c) }
class CustomerAccount {
  name, email
  money: Money
  updateEmail(); displayName()
  charge(a) { money.charge(a) }   // temporary delegation
}

go deeper

for a junior

Name the visible smells — vague name, many unrelated methods, huge test setup — and say the fix is Extract Class into focused classes.

for a middle

Add LCOM as the structural metric (lower is better, LCOM4 counts connected components) and describe Extract Class with a delegating facade so callers keep working.

for a senior

Combine metric, version-control hotspot evidence and smells; describe the incremental releasable sequence and the metric's distortions (constructors, getters, DI fields).

for a principal

Frame it as portfolio-level investment: prioritize hotspots by change frequency times cost, tie boundaries to team ownership, and set exit criteria so refactoring stops when the risk-weighted return does.

## Part 1 — Detecting low cohesion No single measure is authoritative. Use three families of evidence and look for agreement. ### A. Structural metrics (LCOM family) **LCOM = Lack of Cohesion of Methods.** Higher = worse (it measures *lack* of cohesion; people constantly get the direction backwards). - **LCOM1 (Chidamber & Kemerer)** — count method pairs sharing no field, minus pairs that do share one (floored at zero). Crude and easy to game. - **LCOM4** — the most usable variant: build an undirected graph whose nodes are methods; connect two methods if they access a common field **or** one calls the other. Count the **connected components**. `LCOM4 = 1` means the class hangs together; `LCOM4 = 3` means it is really three classes, and the components tell you exactly where to cut. - **LCOM (HS / Henderson-Sellers)** — a normalized 0..1-ish value based on the average number of methods accessing each field; 0 is perfect cohesion, values above ~1 suggest a split. - **TCC / LCC** (Tight/Loose Class Cohesion) — fraction of method pairs that are directly (or transitively) connected through shared attributes; higher is better here. **Where these lie to you** — the caveats that separate a real practitioner from someone quoting a metric: - Trivial **getters/setters** artificially connect or disconnect components. - **Constructors** touch every field and can make a bad class look perfectly cohesive (many tools exclude constructors for this reason). - **Injected dependencies** stored as fields but used by disjoint method groups is precisely the multi-component case — often the *true* positive you want. - **Framework callbacks** (`onCreate`, event handlers) and classes whose state lives outside them (stateless services holding only collaborators) score badly or meaninglessly. - A metric never tells you *whether the split is worth it*; it tells you *where a split is possible*. ### B. Historical / change-coupling evidence (usually the strongest signal) Mine version control: - **Change frequency** — hotspots edited far more than the rest of the codebase. - **Number of distinct authors/teams** — many owners on one file means many reasons to change and coordination cost. - **Ticket/feature diversity** — if commits touching the file belong to unrelated epics, the file mixes concerns by definition. This is a direct, empirical measurement of SRP's "one reason to change". - **Merge conflict rate** on that file. ### C. Human/structural smells - Cannot be named without "and"/"Manager"/"Util". - **Divergent Change** smell: one class changed for many different reasons (the direct symptom of low cohesion). Its twin, **Shotgun Surgery** (one change touches many classes), signals the opposite failure — over-splitting or misplaced responsibility. - Tests require large, unrelated setup, or the test file itself splits into unrelated `describe` blocks. - Imports spanning unrelated technical areas (crypto + HTTP + SQL + date formatting). - Large numbers of fields where any given method uses only two or three. ## Part 2 — The refactoring sequence 1. **Establish a safety net.** If the class is important and undertested, add *characterization tests* (tests that pin current behavior, right or wrong) before touching structure. 2. **Identify the clusters.** From the LCOM4 components, the field-usage table, or simply by reading: which methods and which fields belong to the same story? 3. **Extract Class** (Fowler): create a new class named after the cluster's actual purpose; move its fields and methods across. Where a whole group of behavior plus its data moves, this is sometimes staged as *Extract Delegate*, *Move Field*, then *Move Method*. 4. **Keep the old class as a delegating facade.** The original keeps a reference to the new object and forwards the old method signatures. Nothing calling it breaks — this makes the refactor releasable at any point. 5. **Migrate callers incrementally** to talk to the extracted class directly. Deprecate the forwarding methods. 6. **Remove the dead delegation** once callers are migrated, and re-check the metrics/tests. 7. **Related moves**, depending on the shape found: *Replace Conditional with Polymorphism* (fixes logical cohesion), *Move Method* to the Information Expert that owns the data, *Extract Interface* to narrow what clients see (this reduces coupling in the same stroke), *Introduce Parameter Object* / *Preserve Whole Object* when a cluster of parameters keeps travelling together (that cluster is often a missing cohesive class), and *Replace Data Value with Object* for primitive-obsessed fields. ## Part 3 — Judgement: when NOT to split - The class is **stable** — rarely changed, well tested, understood. Low cohesion costs you mainly at change time; a frozen class costs little. - The split would create an **anemic** pair: data on one side, all behavior on the other, with getters flying across — that usually violates Information Expert and worsens coupling. - The seam does not exist yet: if the clusters overlap heavily on shared mutable state, splitting produces two classes with a chatty, fragile contract. Untangle the state first. - Cost/benefit: refactoring is an investment justified by expected future change. Base the decision on the hotspot data, not on the metric alone. ## Interview framing A strong answer explicitly separates **evidence** (metrics + history + smells, with the caveats), **mechanics** (Extract Class behind a delegating facade, incremental, always releasable), and **judgement** (when the split is not worth it). Quoting LCOM alone, without knowing that higher means worse and that constructors distort it, is a classic trap.

  • Does LCOM higher or lower mean better cohesion?
    Lower is better — LCOM measures the *lack* of cohesion. LCOM4 = 1 is the ideal (one connected component); values above 1 mean the class decomposes into that many independent parts. TCC/LCC are the inverse convention, where higher is better.
  • Why is version-control history often better evidence than a static metric?
    Because cohesion ultimately concerns reasons to change, and history records the actual reasons: which tickets, which teams, and how often each file changed. A static metric can only see structure, so it misses concerns that are entangled through behavior rather than fields, and it flags harmless cases like framework callback classes.
  • How do you keep such a refactor safe in a large codebase?
    Characterization tests first, then Extract Class with the original retained as a delegating facade so every caller keeps compiling. Ship in small releasable steps, migrate callers gradually behind deprecations, and only delete the forwarding layer once no caller remains.

saying these in an interview costs you the question

  • Saying a high LCOM value means good cohesion
  • Trusting LCOM blindly without excluding constructors/getters or accounting for injected dependencies
  • Doing a big-bang split that changes all callers at once instead of extracting behind a delegating facade
  • Splitting until you get an anemic data class plus a procedural manipulator class
  • Refactoring a stable, rarely-touched class purely because a metric is red

context

resolving links…