Spaghetti Code

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

Spaghetti code

Loading interactive demo…

One class from a small fixture program drawn as a box of fields over methods, and a detector reading the flow off it: for every method the fields it writes and reads, and for every field one method writes and another reads, whether a third method calls the two in turn. When one does, the field is a message between two steps of a routine, a hand-off hidden in the object; a boolean tested in an if is a flag. The refactor knob applies Pass Along one field at a time and the detector reads the class again. The refactorings are the library's own toy forms: the producer returns the value, each consumer takes it as a parameter, and the orchestrator hands it down its body.Take the tour, or press and explore. Legend:

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

The member in progress and the edge being read.
A method writing a field (method to field) or reading one (field to method); dashed until the detector has listed it.
A hand-off, writer to field to reader, with a method that calls both in turn; and a flag field.
A channel in the open: a value handed in as a parameter, or handed back as a return, between a method and its caller.

methods counts the methods read, hidden the hand-offs with a via and what the original had, flags the tested booleans among them, explicit the parameters and returns in the open, verdict spaghetti or straight, member who hands the tapped member what. 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

A hand-off has three parts and a tell. One method writes a field, another reads it, and that is an edge; every class with state has some. The tell is a third method that calls the writer and then the reader as steps of its own body. Then the field is not the object's state but a message between two steps of one routine, and the only place the order is written down is that third method. The detector names it the via, and only edges with a via count as hidden.

Why the savings account is clean. Its deposit writes the balance and its available reads it, so the edge is there, but nothing in the class calls the two in turn: callers outside do, in whatever order they like, and the balance really is what the account remembers between those calls. That is what a field is for. The control has two edges, no via, and a straight verdict.

Flags are the worst case. A boolean one step sets and a later step tests in an if is control flow hidden in a field: the decision is made in one place and obeyed in another, with nothing in between to say so. The dispatch scenario's ok is one; the detector tones it and counts it apart, and Pass Along turns it into a return value and a parameter like any other.

The cohesion paradox. By the measure in the cohesion lesson a spaghetti object looks good: every method touches gross or tax, so LCOM4 joins them into one group and calls the class cohesive. The count sees that the methods share fields; it cannot see that the sharing is a relay. That is why this lesson rests on cohesion and then asks a different question of the same facts: not whether the methods share a field, but whether something sequences them through it.

The cure in its toy form. Pass Along takes one field at a time: the producer declares it as a local and returns it, each consumer takes it as a trailing parameter, and the orchestrator binds the return with a let and hands it to each call. The value now flows down one method body where a reader can see it. The library's form demands a lone orchestrator, a producer that returns nothing yet, and consumers that never write the field; real code negotiates several callers, early returns, and fields that are half state and half message.

The control. A detector that only ever says spaghetti is not a detector. The savings account has fields written in one method and read in another, and passes; the verdict rests on the via, not on the sharing.

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

Writes and reads are read once per method through the dead code detector's field scan, which already knows that an assignment's target is the one access that is a write; a method that reads the field it writes, as sumHours does with gross, lists it on both sides.
Every writer, field, reader triple is an edge, across two different methods, in writer order; the via is the first other method whose own calls include both ends, or empty when none does.
A flag is a boolean field tested in an if or a while anywhere in the class; a number compared in an if, like gross against a threshold, is not one.
Explicit counts the channels already open: parameters plus non-void returns over every method, so each Pass Along step shows as hidden edges turning into explicit ones in the same trace.
The verdict reads one number. A single hand-off with a via is spaghetti; the edge count and the flags are reported, not judged.

Spaghetti was first a word for control flow: programs whose jumps crossed until no reader could follow a thread. Structured programming took the jumps away, and the name survived because the tangle moved. In object-oriented code it lives in data flow: a routine chopped into methods that take nothing and return nothing, handing results to each other through the object's fields, with flags for the decisions. Every method touches the shared fields, so a cohesion count calls the class fine. This lesson reads the flow off such a class, names each hidden hand-off, and passes the values along.

1966Corrado Böhm and Giuseppe Jacopini - 'Flow Diagrams, Turing Machines and Languages with Only Two Formation Rules' in Communications of the ACM shows every flowchart can be rewritten with sequence, selection, and iteration alone: the jumps are never needed.source
1968Edsger W. Dijkstra - 'Go To Statement Considered Harmful', a letter in Communications of the ACM, argues that unrestricted jumps make a program's progress impossible to describe, and sets off the structured programming debate.source
1977Guy L. Steele Jr. - 'Macaroni is Better than Spaghetti' is one of several 1977 papers using the pasta image for tangled control flow; by 1978 a programming primer describes programs with 'the same clean logical structure as a plate of spaghetti'.source
1998Brown, Malveau, McCormick, and Mowbray - AntiPatterns: Refactoring Software, Architectures, and Projects in Crisis catalogues Spaghetti Code as an object-oriented shape: process-named objects whose methods take no parameters and work through shared fields.source

Used today

Code review - A method with no parameters and no return value that still does work is the first thing a reviewer asks about: where does its result go, and who depends on the order of calls?
Refactoring to pure functions - Passing each value along as a parameter and a return turns steps into functions of their inputs, which can be tested one at a time and reordered safely.
Data-flow linting - Analysers that track definitions and uses across methods report a field written in one method and read in another as a cross-method dependency, the edge this lesson draws.

Did you know

The pasta kept coming: lasagna code is a system of too many layers, each one hard to change without the others, and ravioli code is a system of so many tiny self-contained objects that nobody can see how they fit together.source
Dijkstra submitted his 1968 letter as 'A Case against the GO TO Statement'. The editor, Niklaus Wirth, turned the article into a letter to get it printed sooner and gave it the title that became a template for a generation of essays.source
Böhm and Jacopini's second normal form needs only two rules, composition and iteration; selection can be simulated with a flag and a loop, which is exactly the hidden control flow this lesson flags when it finds a boolean tested across methods.source
By a cohesion count a spaghetti object scores well: every method touches the shared fields, so LCOM finds one tightly knit group. The cohesion lesson's shared logger fails the same way, which is why this lesson counts hand-offs instead.source

class

refactororiginal

methods: 4 of 4
hidden: 3
flags: 0
explicit: 0
verdict: spaghetti: 3 hidden hand-offs
member: hours: read by sumHours

hidden-flow.ts

import { DeadCode } from "./dead-code";import type { ClassDecl, Method } from "./program";import { ProgramQuery } from "./program-query";import type { Tracer } from "../trace";import type { SmellDetector } from "./detector";/** One hidden hand-off: `from` assigns the field, `to` reads it, and `via` is the method that calls both, when one does. */export interface HiddenEdge {  readonly field: string;  readonly from: string;  readonly to: string;  /** The first other method whose own calls include both ends; null when nothing in the class sequences them. */  readonly via: string | null;  /** The field is a boolean tested in an `if` or a `while`: hidden control flow. */  readonly flag: boolean;}export interface HiddenFlowReport {  readonly className: string;  /** Every writer, field, reader triple across two methods, writers in declaration order. */  readonly edges: readonly HiddenEdge[];  /** The distinct flag fields that take part in a hand-off, in edge order. */  readonly flags: readonly string[];  /** The channels already in the open: parameters plus non-void returns over the class's methods. */  readonly explicit: number;  /** The edges with a via: values passed between steps of one routine through a field. */  readonly passed: number;  readonly smelly: boolean;}/** What one method does with the class's fields through `this`. */interface MethodFlow {  readonly method: Method;  readonly writes: readonly string[];  readonly reads: readonly string[];  readonly calls: readonly string[];}/** * Spaghetti code, in the form Brown et al. name in AntiPatterns (1998): * a routine chopped into methods that take nothing and return nothing, * handing their results to each other through the object's fields and * flags. The class looks cohesive, since every method touches the shared * fields and LCOM is low, but no reader can follow the flow: a value * written in one method is read in another, and only the method that * calls both in order knows that the first must run before the second. * * The detector reads the flow off the class. Per method it lists the * fields assigned through `this` and the fields read through `this` * (an assignment's own target is the one access that is a write, as * `DeadCode.scan` reports it). Every writer, field, reader triple across * two different methods is a hidden edge. The tell is `via`: the first * other method whose own calls include both the writer and the reader. * With a via, the field is a message between two steps of one routine, * which a return value and a parameter should carry instead; the edge is * a hand-off and counts toward `passed`. Without one (an account's * `deposit` and `available` share `balance`, but nothing calls both) the * field is the object's state, and the edge is what an object is for. A * `flag` is a boolean field tested in an `if` or a `while` anywhere in * the class: the classic hidden control flow, where one step decides and * a later step obeys. `explicit` counts the channels already in the open, * parameters and non-void returns, so the cure shows as hidden edges * turning into explicit ones. * * Time: O(s + m^2 * f) over s statements, m methods, and f fields of the * class. Space: O(m * f) for the per-method field lists. */export class HiddenFlow implements SmellDetector<ClassDecl, HiddenFlowReport> {  constructor(private readonly tracer?: Tracer) {}  analyze(cls: ClassDecl): HiddenFlowReport {    const flows = cls.methods.map((method) => HiddenFlow.flowOf(method));    const flagFields = HiddenFlow.flagFields(cls);    flows.forEach((flow, index) =>      // ▸ a method: the fields it writes and reads, what it takes and returns      this.tracer?.emit("method", {        className: cls.name,        name: flow.method.name,        params: flow.method.params.length,        returns: flow.method.returns,        writes: [...flow.writes],        reads: [...flow.reads],        index,        count: flows.length,      }),    );    const edges: HiddenEdge[] = [];    for (const writer of flows) {      for (const field of writer.writes) {        for (const reader of flows) {          if (reader === writer || !reader.reads.includes(field)) continue;          const via = flows.find((other) => other !== writer && other !== reader && other.calls.includes(writer.method.name) && other.calls.includes(reader.method.name));          const edge: HiddenEdge = { field, from: writer.method.name, to: reader.method.name, via: via ? via.method.name : null, flag: flagFields.includes(field) };          edges.push(edge);          // ▸ one method writes a field another reads; a via makes it a hand-off          this.tracer?.emit("edge", { field: edge.field, from: edge.from, to: edge.to, via: edge.via ?? "", flag: edge.flag, count: edges.length });        }      }    }    const passed = edges.filter((edge) => edge.via !== null).length;    const flags = [...new Set(edges.filter((edge) => edge.via !== null && edge.flag).map((edge) => edge.field))];    const explicit = cls.methods.reduce((n, method) => n + method.params.length + (method.returns === "void" ? 0 : 1), 0);    const smelly = passed > 0;    // ▸ hidden hand-offs, flags, and channels in the open    this.tracer?.emit("verdict", { className: cls.name, methods: cls.methods.length, edges: edges.length, passed, flags: [...flags], explicit, smelly });    return { className: cls.name, edges, flags, explicit, passed, smelly };  }  /** The fields a method assigns and reads through `this`, each in first-use order, and the methods it calls on `this`. */  private static flowOf(method: Method): MethodFlow {    const writes: string[] = [];    const reads: string[] = [];    ProgramQuery.walk(method.body, (stmt) =>      DeadCode.scan(        stmt,        () => undefined,        (field, written) => {          if (field.target.kind !== "this") return;          const list = written ? writes : reads;          if (!list.includes(field.name)) list.push(field.name);        },      ),    );    return { method, writes, reads, calls: ProgramQuery.selfCalls(method.body) };  }  /** The boolean fields of the class that some `if` or `while` test reads through `this`, anywhere in the class. */  private static flagFields(cls: ClassDecl): readonly string[] {    const booleans = new Set(cls.fields.filter((field) => field.type === "boolean").map((field) => field.name));    const flags: string[] = [];    for (const method of cls.methods) {      ProgramQuery.walk(method.body, (stmt) => {        if (stmt.kind !== "if" && stmt.kind !== "while") return;        ProgramQuery.visitExpr(stmt.test, (e) => {          if (e.kind === "field" && e.target.kind === "this" && booleans.has(e.name) && !flags.includes(e.name)) flags.push(e.name);        });      });    }    return flags;  }}

hidden =: 3
explicit =: 0

Payroll: 3 hidden hand-offs; 0 in the open. Still spaghetti.

import { DeadCode } from "./dead-code";import type { ClassDecl, Method } from "./program";import { ProgramQuery } from "./program-query";import type { Tracer } from "../trace";import type { SmellDetector } from "./detector";/** One hidden hand-off: `from` assigns the field, `to` reads it, and `via` is the method that calls both, when one does. */export interface HiddenEdge { readonly field: string; readonly from: string; readonly to: string; /** The first other method whose own calls include both ends; null when nothing in the class sequences them. */ readonly via: string | null; /** The field is a boolean tested in an `if` or a `while`: hidden control flow. */ readonly flag: boolean;}export interface HiddenFlowReport { readonly className: string; /** Every writer, field, reader triple across two methods, writers in declaration order. */ readonly edges: readonly HiddenEdge[]; /** The distinct flag fields that take part in a hand-off, in edge order. */ readonly flags: readonly string[]; /** The channels already in the open: parameters plus non-void returns over the class's methods. */ readonly explicit: number; /** The edges with a via: values passed between steps of one routine through a field. */ readonly passed: number; readonly smelly: boolean;}/** What one method does with the class's fields through `this`. */interface MethodFlow { readonly method: Method; readonly writes: readonly string[]; readonly reads: readonly string[]; readonly calls: readonly string[];}/** * Spaghetti code, in the form Brown et al. name in AntiPatterns (1998): * a routine chopped into methods that take nothing and return nothing, * handing their results to each other through the object's fields and * flags. The class looks cohesive, since every method touches the shared * fields and LCOM is low, but no reader can follow the flow: a value * written in one method is read in another, and only the method that * calls both in order knows that the first must run before the second. * * The detector reads the flow off the class. Per method it lists the * fields assigned through `this` and the fields read through `this` * (an assignment's own target is the one access that is a write, as * `DeadCode.scan` reports it). Every writer, field, reader triple across * two different methods is a hidden edge. The tell is `via`: the first * other method whose own calls include both the writer and the reader. * With a via, the field is a message between two steps of one routine, * which a return value and a parameter should carry instead; the edge is * a hand-off and counts toward `passed`. Without one (an account's * `deposit` and `available` share `balance`, but nothing calls both) the * field is the object's state, and the edge is what an object is for. A * `flag` is a boolean field tested in an `if` or a `while` anywhere in * the class: the classic hidden control flow, where one step decides and * a later step obeys. `explicit` counts the channels already in the open, * parameters and non-void returns, so the cure shows as hidden edges * turning into explicit ones. * * Time: O(s + m^2 * f) over s statements, m methods, and f fields of the * class. Space: O(m * f) for the per-method field lists. */export class HiddenFlow implements SmellDetector<ClassDecl, HiddenFlowReport> { constructor(private readonly tracer?: Tracer) {} analyze(cls: ClassDecl): HiddenFlowReport { const flows = cls.methods.map((method) => HiddenFlow.flowOf(method)); const flagFields = HiddenFlow.flagFields(cls); flows.forEach((flow, index) => // ▸ a method: the fields it writes and reads, what it takes and returns this.tracer?.emit("method", { className: cls.name, name: flow.method.name, params: flow.method.params.length, returns: flow.method.returns, writes: [...flow.writes], reads: [...flow.reads], index, count: flows.length, }), ); const edges: HiddenEdge[] = []; for (const writer of flows) { for (const field of writer.writes) { for (const reader of flows) { if (reader === writer || !reader.reads.includes(field)) continue; const via = flows.find((other) => other !== writer && other !== reader && other.calls.includes(writer.method.name) && other.calls.includes(reader.method.name)); const edge: HiddenEdge = { field, from: writer.method.name, to: reader.method.name, via: via ? via.method.name : null, flag: flagFields.includes(field) }; edges.push(edge); // ▸ one method writes a field another reads; a via makes it a hand-off this.tracer?.emit("edge", { field: edge.field, from: edge.from, to: edge.to, via: edge.via ?? "", flag: edge.flag, count: edges.length }); } } } const passed = edges.filter((edge) => edge.via !== null).length; const flags = [...new Set(edges.filter((edge) => edge.via !== null && edge.flag).map((edge) => edge.field))]; const explicit = cls.methods.reduce((n, method) => n + method.params.length + (method.returns === "void" ? 0 : 1), 0); const smelly = passed > 0; // ▸ hidden hand-offs, flags, and channels in the open this.tracer?.emit("verdict", { className: cls.name, methods: cls.methods.length, edges: edges.length, passed, flags: [...flags], explicit, smelly }); return { className: cls.name, edges, flags, explicit, passed, smelly }; } /** The fields a method assigns and reads through `this`, each in first-use order, and the methods it calls on `this`. */ private static flowOf(method: Method): MethodFlow { const writes: string[] = []; const reads: string[] = []; ProgramQuery.walk(method.body, (stmt) => DeadCode.scan( stmt, () => undefined, (field, written) => { if (field.target.kind !== "this") return; const list = written ? writes : reads; if (!list.includes(field.name)) list.push(field.name); }, ), ); return { method, writes, reads, calls: ProgramQuery.selfCalls(method.body) }; } /** The boolean fields of the class that some `if` or `while` test reads through `this`, anywhere in the class. */ private static flagFields(cls: ClassDecl): readonly string[] { const booleans = new Set(cls.fields.filter((field) => field.type === "boolean").map((field) => field.name)); const flags: string[] = []; for (const method of cls.methods) { ProgramQuery.walk(method.body, (stmt) => { if (stmt.kind !== "if" && stmt.kind !== "while") return; ProgramQuery.visitExpr(stmt.test, (e) => { if (e.kind === "field" && e.target.kind === "this" && booleans.has(e.name) && !flags.includes(e.name)) flags.push(e.name); }); }); } return flags; }}