familienarchiv/CODESTYLE.md

Code Style Guide

This document defines the coding standards for the Familienarchiv project. It applies to both the Java backend and the TypeScript/Svelte frontend. When in doubt, prefer code that a competent developer can read and understand without explanation.

---

Clean Code (Uncle Bob)

These are principles, not laws. Apply judgment.

Names reveal intent

A name should tell you why something exists, what it does, and how it is used — without needing a comment to explain it.

// Bad
int d; // elapsed time in days
List<Document> list2;

// Good
int elapsedDays;
List<Document> receivedDocuments;

• No abbreviations unless universally understood (id, url, dto).
• Boolean variables and methods should read as yes/no questions: isEnabled, hasFile, canWrite.
• Avoid redundant context: inside class Document, write getTitle() not getDocumentTitle().

Functions do one thing

A function that does one thing can rarely be meaningfully subdivided. If you can extract a chunk with a name that isn't just a restatement of what it does, it should probably be its own function.

// Bad — validates, transforms, and persists
public Document saveDocument(DocumentUpdateDTO dto) {
    if (dto.getTitle() == null) throw new DomainException(...);
    String cleaned = dto.getTitle().strip();
    Document doc = documentRepository.findById(...).orElseThrow(...);
    doc.setTitle(cleaned);
    return documentRepository.save(doc);
}

// Good — one responsibility per method; caller orchestrates
public Document updateDocument(UUID id, DocumentUpdateDTO dto) {
    validateTitle(dto.getTitle());
    Document doc = getById(id);
    applyUpdate(doc, dto);
    return documentRepository.save(doc);
}

Small functions, minimal parameters

• Functions longer than ~20 lines are a signal to look for a natural split.
• Aim for ≤ 3 parameters. More than 3 is a sign the function is doing too much, or parameters should be grouped into an object.
• Never use boolean flag arguments — they announce the function does two things:

// Bad
renderDocument(doc, true);  // what does true mean?

// Good
renderDocumentWithPreview(doc);
renderDocumentWithoutPreview(doc);

Guard clauses over deep nesting

Return or throw early for preconditions. Keep the happy path at the lowest nesting level.

// Bad
public Document getDocument(UUID id, AppUser user) {
    if (id != null) {
        Document doc = repository.findById(id).orElse(null);
        if (doc != null) {
            if (user.canRead(doc)) {
                return doc;
            }
        }
    }
    return null;
}

// Good
public Document getDocument(UUID id, AppUser user) {
    if (id == null) throw DomainException.notFound(...);
    Document doc = repository.findById(id)
        .orElseThrow(() -> DomainException.notFound(...));
    if (!user.canRead(doc)) throw DomainException.forbidden(...);
    return doc;
}

Comments: only for why, never for what

Code explains what it does. Comments explain why a non-obvious decision was made.

// Bad — restates the code
// set auth cookie
cookies.set('auth_token', authHeader, { path: '/' });

// Good — explains a non-obvious constraint
// secure: false until the deployment is served over HTTPS
cookies.set('auth_token', authHeader, { path: '/', secure: false });

If you feel compelled to write a comment that explains what the code does, rewrite the code until it doesn't need one.

No dead code

Remove commented-out code, unused variables, unused imports, and unreachable branches. Version control is the history — dead code in the file is noise.

Command-query separation

A function either does something (command) or answers something (query) — not both.

// Bad — modifies state and returns a value
function addTagAndReturnCount(tag: string): number { ... }

// Good — separate concerns
function addTag(tag: string): void { ... }
function getTagCount(): number { ... }

---

DRY vs KISS — KISS wins

DRY (Don't Repeat Yourself): every piece of knowledge has a single, authoritative representation.

KISS (Keep It Simple, Stupid): prefer the simplest solution that works.

When they conflict, KISS wins. Do not create an abstraction to eliminate duplication unless the abstraction has a clear, stable name and genuinely reduces cognitive load.

Practical rules

Extract when:

• The same logic appears in 3+ places and it has a meaningful name that isn't just a description of the lines it replaces.
• The extracted unit is independently testable.
• The abstraction makes the call site more readable, not less.

Don't extract when:

• Two things look similar but might diverge independently — coupling them through an abstraction would make future changes harder.
• The extracted function would be used exactly once.
• Naming the abstraction requires a long or awkward name.

// Three similar lines — do NOT abstract prematurely
const sentYearRange   = yearRange(sentDocuments);
const receivedYearRange = yearRange(receivedDocuments);

// yearRange() is worth extracting because it has a clear name,
// is used in multiple places, and is independently testable.
// But if it were only used once, keep it inline.

---

SOLID Principles

Applied to this stack.

S — Single Responsibility

Each class, service, or component has one reason to change. In practice:

• Backend: Controllers receive HTTP, delegate everything to services. Services contain business logic, never touch another domain's repository directly.
• Frontend: Components render UI. Server files (+page.server.ts) load and validate data. Don't put business logic in Svelte components.
• Wrong: A DocumentService that also manages user sessions.
• Right: DocumentService owns documents; UserService owns users; each is ignorant of the other's internal details.

O — Open/Closed

Code should be open for extension and closed for modification. Prefer adding new code over editing existing code to support new behavior.

// Bad — adding a new export format requires editing this method
public byte[] export(String format) {
    if (format.equals("csv")) { ... }
    else if (format.equals("pdf")) { ... }  // added later, modifies existing method
}

// Good — each format is a separate implementation
public interface DocumentExporter {
    byte[] export(List<Document> documents);
}
public class CsvExporter implements DocumentExporter { ... }
public class PdfExporter implements DocumentExporter { ... }

In practice: when adding a variant of existing behavior, reach for a new class/function before editing an existing one.

L — Liskov Substitution

Subtypes must be usable wherever the parent type is expected, without breaking behavior. Concretely:

• If you extend a service or implement an interface, the subtype must honor the contracts (error cases, return semantics) of the parent.
• Don't override a method to make it a no-op or throw unconditionally — that breaks callers who rely on the contract.

I — Interface Segregation

Don't force callers to depend on methods they don't use. Keep interfaces and services focused.

// Bad — DocumentService exposed to ImportService even though import only needs findOrCreate
public class MassImportService {
    private final DocumentService documentService; // 40+ methods, only 2 needed
}

// Good — expose only what's needed via a targeted service method or a narrow interface
public class MassImportService {
    private final PersonService personService;     // only needs findOrCreateByName
    private final TagService tagService;           // only needs findOrCreate
}

D — Dependency Inversion

High-level modules should not depend on low-level modules. Both should depend on abstractions.

• Backend: Spring's @Autowired / constructor injection handles this. Always inject interfaces or Spring beans, never instantiate services with new inside a controller or service.
• Frontend: Pass data into components via props rather than fetching it inside the component. Components should receive data; server files should supply it.

// Bad — component fetches its own data (depends on network/fetch implementation)
onMount(async () => {
    persons = await fetch('/api/persons').then(r => r.json());
});

// Good — data flows in via props from the server load function
let { data } = $props();  // data.persons supplied by +page.server.ts

---

Formatting and Style Specifics

These complement the principles above with project-specific conventions.

Both Java and TypeScript

• One concept per line — don't chain side-effects.
• No magic numbers — extract named constants.
• Fail fast: validate inputs at the boundary (controller / server load), trust internal code.

Java (backend)

• Use DomainException static factories for all domain errors — never throw raw RuntimeException.
• @Transactional only on write methods, not reads.
• Entities use @Builder — construct with builder pattern, not setters, in tests.
• Avoid Optional.get() without orElseThrow — always provide a meaningful exception.

TypeScript / Svelte (frontend)

• $derived over $effect for computed values — effects are for side-effects only.
• Check !result.response.ok for API errors, not result.error (see CLAUDE.md).
• Prefer typed API client calls over raw fetch — use raw fetch only for multipart uploads.
• Svelte component logic in <script>, layout/styles in template — no business logic in markup.

---

Svelte 5 — Specific Rules

These rules are enforced by ESLint (eslint-plugin-svelte). Knowing why they exist prevents the need to fix violations after the fact.

Always key {#each} blocks

Without a key, Svelte tracks list items by array position. When items are added, removed, or reordered, Svelte patches DOM nodes in-place from the top — it never moves the correct node. Component-local state (counters, animation state, focus) becomes permanently attached to the wrong item. This is a silent data integrity bug, not a crash.

<!-- Bad — position-based tracking; reordering silently corrupts local state -->
{#each documents as doc}
    <DocumentCard {doc} />
{/each}

<!-- Good — identity-based; each node follows its data through reorders -->
{#each documents as doc (doc.id)}
    <DocumentCard {doc} />
{/each}

Use (item.id) when items have a stable ID. Use the loop index (i) only for static lists that will never be reordered. Use (item) for primitive lists.

Use $derived for computed values, never $state + $effect

$effect is for side effects (DOM calls, network, logging). Using it to assign a computed value introduces a timing problem: $derived updates synchronously before the render, while $effect runs after the render — meaning the component briefly displays a stale value. It also triggers a second reactive pass, doubling the work.

<!-- Bad — stale value during render; extra reactive cycle; unclear intent -->
<script>
    let fullName = $state('');
    $effect(() => {
        fullName = `${person.firstName} ${person.lastName}`;
    });
</script>

<!-- Good — synchronous, single-pass, intent is obvious -->
<script>
    const fullName = $derived(`${person.firstName} ${person.lastName}`);
</script>

Use $derived.by(() => { ... }) when the computation needs multiple statements.

Use Svelte reactive collections, not plain JS ones

Svelte 5's reactivity tracks object references, not mutations. When you call .set() on a plain Map or .set() on a plain URLSearchParams, the reference doesn't change — Svelte never notices, and the UI goes silently stale.

SvelteMap, SvelteSet, and SvelteURLSearchParams from svelte/reactivity wrap the native classes and hook into Svelte's dependency tracker. Every mutation notifies the reactive graph; every read registers a dependency.

<!-- Bad — mutations are invisible to Svelte; derived values never update -->
<script>
    const freq = new Map<string, number>();
    freq.set('key', 1); // Svelte does not see this
</script>

<!-- Good — mutations are tracked; all dependents re-run correctly -->
<script>
    import { SvelteMap } from 'svelte/reactivity';
    const freq = new SvelteMap<string, number>();
    freq.set('key', 1); // Svelte tracks this
</script>

The same applies to URLSearchParams in reactive contexts — use SvelteURLSearchParams.