ADR-033: Don’t Repeat Yourself

Status

Accepted

Context

Smarter has dozens of resource kinds, and each one needs much the same parts: a model with an owner, a manifest, a broker, an api, a web console list page, url names, tests and docs. Code like that invites copy and paste. A copy works the day it is made, and then drifts: a bug is fixed in one copy and not the others, or a rule (a permission check, a log prefix, a cache key) changes in one place and stays old everywhere else.

Most of the codebase already avoids this with shared base classes and helpers, for example:

This ADR states the rule that those decisions have in common.

Decision

Every piece of knowledge in Smarter has one authoritative home. Code, tests and docs refer to it instead of restating it.

  1. Extend what exists before writing something new. Before adding a helper, base class, constant or React hook, search for one that does the job, and extend it if it almost does.

  2. The third copy is the signal. Two similar blocks of code may stay as they are. When a third one appears, move the shared part into a function, base class or mixin, and change all three to use it.

  3. Shared code lives with its scope. Code that several Django apps use goes in smarter/lib (framework) or the app that owns the concept (for example account for ownership). Code that several React apps use goes in smarter-common. An app never imports another app’s private helpers to avoid writing its own.

  4. Generate, don’t transcribe. A value that can be derived from another (a url name from its view, a JSON Schema or TypeScript type from a Pydantic model, a choice list from an enum) is derived in code, and a test fails when a committed copy is out of date (ADR-031).

  5. Constants and configuration are defined once. A string or number with a meaning (a namespace, a limit, a setting’s name) is a named constant or a setting, not a literal repeated in several modules (ADR-007).

  6. Docs link instead of repeating. A page that needs a fact explained elsewhere links to it with :doc: or :ref:. Technical reference comes from docstrings through autodoc (ADR-029), not from prose that copies them.

  7. Don’t merge things that only look alike. Duplication is a problem when two copies must change together. Two pieces of code that happen to look the same today, but answer to different rules, stay separate. A shared helper full of flags for each caller is worse than the copies it replaced.

  8. Tests may repeat themselves for clarity. Shared setup belongs in base classes and fixtures, but each test’s own inputs and assertions stay visible in the test, even when its neighbours look much the same.

Alternatives Considered

  • No rule; reviewers judge each case. This is how the copies that the shared classes later replaced came about. A rule gives contributors and coding agents the same answer.

  • Remove every duplicate as soon as it appears. Abstracting from two examples guesses at the shape of the third, and often guesses wrong. The rule of three waits for evidence.

Consequences

  • Positive:

    • A fix or a change of rule is made once, and reaches every caller.

    • New resource kinds are mostly configuration of existing base classes, so they are quick to add and behave like the others.

    • Less code to read, test and keep covered.

  • Negative:

    • Shared code has many callers, so changing it needs more care, and its tests must cover each kind of caller.

    • Indirection: understanding a resource means reading its base classes too.

    • Judging rules 2 and 7 takes experience, and contributors will sometimes disagree.

    • Older modules that predate a shared helper remain until someone changes them for another reason.