docs: add CODESTYLE.md with Clean Code, DRY/KISS, and SOLID principles
Covers naming, function design, guard clauses, comment policy, command-query separation, DRY vs KISS trade-offs (KISS wins), and SOLID applied to the Java backend and TypeScript/Svelte frontend. Linked from CLAUDE.md and COLLABORATING.md. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -8,7 +8,9 @@ This file provides guidance to Claude Code (claude.ai/code) when working with co
|
||||
|
||||
## Collaboration
|
||||
|
||||
See [COLLABORATING.md](./COLLABORATING.md) for the full rules: issue tracking workflow, commit message conventions, the Research → Plan → Implement → Validate cycle, and code style expectations.
|
||||
See [COLLABORATING.md](./COLLABORATING.md) for the full rules: issue tracking workflow, commit message conventions, and the Research → Plan → Implement → Validate cycle.
|
||||
|
||||
See [CODESTYLE.md](./CODESTYLE.md) for coding standards: Clean Code, DRY/KISS trade-offs (KISS wins), and SOLID principles applied to this stack.
|
||||
|
||||
---
|
||||
|
||||
|
||||
261
CODESTYLE.md
Normal file
261
CODESTYLE.md
Normal file
@@ -0,0 +1,261 @@
|
||||
# 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.
|
||||
|
||||
```java
|
||||
// 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.
|
||||
|
||||
```java
|
||||
// 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:
|
||||
|
||||
```java
|
||||
// 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.
|
||||
|
||||
```java
|
||||
// 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.
|
||||
|
||||
```typescript
|
||||
// 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.
|
||||
|
||||
```typescript
|
||||
// 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.
|
||||
|
||||
```typescript
|
||||
// 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.
|
||||
|
||||
```java
|
||||
// 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.
|
||||
|
||||
```java
|
||||
// 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.
|
||||
|
||||
```typescript
|
||||
// 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.
|
||||
@@ -140,9 +140,12 @@ fix(e2e): clear locale cookie when switching back to base language
|
||||
|
||||
When in doubt, commit more often rather than less.
|
||||
|
||||
## Code Style Reminders
|
||||
## Code Style
|
||||
|
||||
See [CODESTYLE.md](./CODESTYLE.md) for the full guide: Clean Code (Uncle Bob), DRY/KISS trade-offs, and SOLID principles applied to this stack.
|
||||
|
||||
Quick reminders:
|
||||
- Pure functions over stateful helpers where possible
|
||||
- No premature abstractions — solve the problem in front of you
|
||||
- No premature abstractions — KISS beats DRY
|
||||
- No backwards-compatibility shims for code that has no callers
|
||||
- Validate at system boundaries only (user input, external APIs)
|
||||
|
||||
Reference in New Issue
Block a user