coreci/atelier
3
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases 28 Wiki Activity

docs(P05): complete examples phase

Browse Source
This commit is contained in:
Jon Chery
2026-08-05 00:33:28 +00:00
parent ebcae21630
commit c91f3e754a
7 changed files with 696 additions and 0 deletions
Download Patch File Download Diff File Expand all files Collapse all files
examples/bad/god-object.md
+107
View File
@@ -0,0 +1,107 @@
# Bad Example: God Object
> A component that violates Atelier principles. Each violation is cited.
## The Code
```typescript
// UserManager.ts — 1,200 lines
class UserManager {
private users: User[] = [];
private cache: Map<string, User> = new Map();
private db: Database;
private emailService: EmailService;
private logger: Logger;
private auditLog: AuditLog;
constructor(db: Database, email: EmailService, logger: Logger, audit: AuditLog) {
this.db = db;
this.emailService = email;
this.logger = logger;
this.auditLog = audit;
}
// CRUD
async createUser(data: UserData): Promise<User> { /* 80 lines */ }
async getUser(id: string): Promise<User> { /* 40 lines */ }
async updateUser(id: string, data: Partial<UserData>): Promise<User> { /* 60 lines */ }
async deleteUser(id: string): Promise<void> { /* 50 lines */ }
async listUsers(page: number): Promise<User[]> { /* 40 lines */ }
// Email
async sendWelcomeEmail(user: User): Promise<void> { /* 50 lines */ }
async sendPasswordReset(user: User): Promise<void> { /* 50 lines */ }
async sendDeletionNotice(user: User): Promise<void> { /* 40 lines */ }
// Auth
async authenticate(email: string, password: string): Promise<boolean> { /* 70 lines */ }
async authorize(userId: string, action: string): Promise<boolean> { /* 60 lines */ }
async hashPassword(password: string): Promise<string> { /* 20 lines */ }
// Cache
private cacheGet(id: string): User | null { /* 20 lines */ }
private cacheSet(user: User): void { /* 20 lines */ }
private cacheInvalidate(id: string): void { /* 20 lines */ }
// Audit
private logAudit(action: string, userId: string): void { /* 30 lines */ }
// Validation
private validateEmail(email: string): boolean { /* 20 lines */ }
private validatePassword(password: string): boolean { /* 20 lines */ }
// Serialization
toJSON(user: User): Record<string, unknown> { /* 30 lines */ }
fromJSON(data: Record<string, unknown>): User { /* 30 lines */ }
// ... 200 more lines of helper methods
}
```
## Violations
### C3 Simplicity (Core)
- A 1,200-line class doing 8 different things (CRUD, email, auth, cache, audit, validation, serialization).
- The class cannot be understood in one read. Complexity is the liability.
- **Fix:** Split into `UserRepository` (CRUD), `UserEmailService` (email), `UserAuthService` (auth), `UserCache` (cache), `UserAuditLogger` (audit), `UserValidator` (validation), `UserSerializer` (serialization).
### C6 Composability (Core)
- The class takes 4 dependencies and does 8 jobs. It is not composable; it is monolithic.
- You cannot reuse the email logic without the DB, the cache, the audit log.
- **Fix:** Each responsibility is its own class. Compose them: `UserEmailService` takes only `EmailService`.
### components.md §1 Single Responsibility (UI/UX, applies to code)
- The class name is `UserManager`. "Manager" is a smell — it manages what? Everything.
- If the name is "Manager," it has no single responsibility.
- **Fix:** Name by responsibility: `UserRepository`, `UserAuthService`. Names that cannot be "And"-ed.
### C4 Locality (Core)
- Cache logic is in the same class as email logic. A change to cache touches the email methods' neighbor.
- Related logic (cache get/set/invalidate) is grouped, but unrelated logic (email) is adjacent.
- **Fix:** `UserCache` is its own class. Cache changes are local to cache.
### C2 Clarity (Core)
- A reader cannot answer "what does `UserManager` do?" in one sentence.
- The class has 20+ methods. The reader must scan all of them to find the one they need.
- **Fix:** Smaller classes with clear names. The name is the documentation.
### Security P2 Least Privilege (Security)
- The class has `db`, `emailService`, `logger`, `auditLog` — all available to all methods.
- `sendWelcomeEmail` has access to `db.delete`. Least privilege is violated.
- **Fix:** Each service has only the dependencies it needs. `UserEmailService` has `EmailService`, not `Database`.
### Testing P2 Independence (Testing)
- To test `sendWelcomeEmail`, you must construct `UserManager` with a real/mock DB, email, logger, audit.
- The test setup is 4 mocks for one method. Independence is violated.
- **Fix:** Test `UserEmailService` with one mock (`EmailService`).
## What This Example Reveals
The "God Object" is the cardinal sin of OOP. It violates C3 (Simplicity), C6 (Composability), C4 (Locality), and C2 (Clarity) simultaneously. Every other principle suffers downstream:
- Testing is hard (T2 Independence).
- Security is loose (S2 Least Privilege).
- Evolution is brittle (a change to email risks cache).
- Review is exhausting (a 1,200-line diff).
The fix is always the same: **decompose by responsibility**. The class name is the test: if it is "Manager," "Handler," or "Helper," it has no single responsibility.
examples/bad/leaky-abstraction.md
+87
View File
@@ -0,0 +1,87 @@
# Bad Example: Leaky Abstraction
> An abstraction that leaks its implementation details, violating Atelier principles. Each violation is cited.
## The Code
```typescript
// UserRepository — "abstracts" the database
class UserRepository {
async findAll(): Promise<UserRow[]> {
// Leaks: returns the raw DB row type, not a domain User
return db.query('SELECT id, email, password_hash, created_at, deleted_at FROM users');
}
async findByEmail(email: string): Promise<UserRow | null> {
// Leaks: the caller must know to filter deleted_at
const rows = await db.query('SELECT * FROM users WHERE email = $1', [email]);
return rows[0] || null;
}
async save(user: UserRow): Promise<void> {
// Leaks: the caller must know the column names and the SQL
await db.query(
'UPDATE users SET email = $1, password_hash = $2, updated_at = now() WHERE id = $3',
[user.email, user.password_hash, user.id]
);
}
}
// Usage — the leak is visible
const repo = new UserRepository();
const user = await repo.findByEmail('jane@example.com');
if (user && !user.deleted_at) { // caller must know about soft delete
user.password_hash = await hash(newPassword); // caller must know the column
await repo.save(user); // caller must know it's an UPDATE
}
```
## Violations
### C6 Composability (Core)
- The abstraction is supposed to hide the database. It does not.
- The caller must know: the row type (`UserRow`), the soft-delete column (`deleted_at`), the password column (`password_hash`), the SQL operation (`UPDATE`).
- The abstraction is a thin wrapper. It composes nothing; it leaks everything.
- **Fix:** Return a domain `User` (no `password_hash`, no `deleted_at`). Hide soft delete (the repo filters it). Hide persistence (the caller calls `save`, not `UPDATE`).
### C2 Clarity (Core)
- The caller's code is unclear: `if (user && !user.deleted_at)` — what is `deleted_at`? Why does the caller check it?
- The abstraction was supposed to clarify. It muddied.
- **Fix:** `repo.findByEmail()` returns `User | null` (already filtered). The caller does not know soft delete exists.
### API P2 Clarity (API, by analogy)
- The repo's API exposes the DB schema in its return types. `UserRow` is a DB concept, not a domain concept.
- The public contract (return type) leaks the private implementation (the table).
- **Fix:** The return type is `User`, a domain type. `UserRow` is internal.
### Data P8 Lifecycle Awareness (Data)
- The soft-delete lifecycle (`deleted_at`) is the repo's concern. The caller should not manage it.
- By exposing `deleted_at`, the repo forces every caller to remember the filter. A forgotten filter is a soft-delete leak.
- **Fix:** The repo filters `deleted_at IS NULL` in every query. The caller never sees `deleted_at`.
### Security P9 Secret Hygiene (Security)
- `password_hash` is in the returned `UserRow`. The caller now has access to the password hash.
- A caller that logs `user` logs the hash. A caller that serializes `user` serializes the hash.
- **Fix:** `User` does not include `password_hash`. Only `UserRepository` and `AuthService` (internal) see it.
### C4 Locality (Core)
- The SQL is in the repo, but the column knowledge (`password_hash`, `deleted_at`) is in the caller.
- A column rename touches the repo AND every caller. Locality is violated.
- **Fix:** Column names are local to the repo. The caller knows only the domain `User`.
### C5 Reversibility (Core)
- Changing the database (e.g., from SQL to NoSQL, or renaming a column) requires touching every caller.
- The abstraction was supposed to make the change local. It does not.
- **Fix:** The repo's interface (`findByEmail`, `save`) is stable. The implementation changes; the callers do not.
## What This Example Reveals
The leaky abstraction is the false promise of encapsulation. The class is named `UserRepository` (suggesting it abstracts persistence), but it returns raw DB rows, exposes lifecycle columns, and leaks secret fields. The abstraction exists in name only.
The cost:
- Every caller must know the DB schema (C6 violated).
- A schema change touches every caller (C5 violated, C4 violated).
- Secret fields leak to callers (Security P9 violated).
- The lifecycle is the caller's burden (Data P8 violated).
The fix is always the same: **the abstraction's public type is the domain type, not the implementation type**. `UserRepository.findByEmail()` returns `User | null`, where `User` has `id`, `email`, `name` — and nothing else. `password_hash`, `deleted_at`, `UserRow` are internal. The caller knows nothing about the database.
examples/bad/silent-error.md
+88
View File
@@ -0,0 +1,88 @@
# Bad Example: Silent Error
> An error-handling pattern that violates Atelier principles. Each violation is cited.
## The Code
```typescript
async function getUser(id: string): Promise<User | null> {
try {
const user = await db.query('SELECT * FROM users WHERE id = $1', [id]);
return user;
} catch (e) {
return null;
}
}
async function processOrder(orderId: string): Promise<void> {
const order = await getOrder(orderId);
if (!order) {
return; // silently do nothing
}
// ... process
}
// Usage in a route
router.get('/users/:id', async (req, res) => {
const user = await getUser(req.params.id);
if (!user) {
res.status(404).json({ error: 'Not found' });
} else {
res.json({ data: user });
}
});
```
## Violations
### Errors P2 Fail Loudly (Errors)
- `catch (e) { return null }` swallows the error. The caller cannot distinguish "user not found" from "database down."
- A database outage returns 404s. The operator never knows. Silent failure.
- **Fix:** Catch and re-throw with context, or return a typed error (`Result<User, Error>`). Never `null` for "an error happened."
### Errors P3 Fail Specifically (Errors)
- `return null` is the least specific response. It could mean: not found, db error, network error, permission error.
- The caller's `if (!user)` cannot distinguish these. The 404 is a lie if the real cause was a 500.
- **Fix:** Return `Result` or throw. The error type/code carries the specificity.
### Errors P1 Errors are Data (Errors)
- `null` is not data. It is the absence of data. Conflating "error" with "absence" loses information.
- The error (a database failure) was data; it was thrown away and replaced with `null`.
- **Fix:** Errors are values. Return the error value, not a sentinel absence.
### Errors P4 Preserve Context (Errors)
- The catch block discards `e`. The stack trace, the error message, the cause — all gone.
- The log has no record. The operator cannot debug.
- **Fix:** Log the error with context. Wrap and re-throw: `throw new Error('getUser failed', { cause: e })`.
### Errors P9 Errors are Logged (Errors)
- The error is not logged. The handling (return null) is the entire response. The log is missing.
- An error that is not logged is an error that cannot be investigated.
- **Fix:** `logger.error({ err: e, userId: id })` before returning/rethrowing.
### Errors P10 Errors Don't Lie (Errors)
- `return null` claims "no user" when the truth may be "database down." The function lies.
- The 404 response claims "not found" when the truth may be "internal error." The API lies.
- **Fix:** The response status must match the actual condition. 500 for server errors, 404 for not found.
### Observability P2 Correlation, P3 Context (Observability)
- No `request_id`. No correlation across services.
- No context in the (missing) log. "What was the user doing?" is unanswerable.
- **Fix:** Propagate `request_id`. Log with path, method, user_id.
### Security P8 Fail Securely (Security)
- The silent failure is fail-open in disguise. If `getUser` fails due to an authz check throwing, the catch returns `null`.
- The caller treats `null` as "not found" and may proceed, or may 404. Either way, the security failure is hidden.
- **Fix:** Distinguish "not found" (404) from "authz error" (403) from "db error" (500). Never collapse them into `null`.
## What This Example Reveals
The silent error is the most common and most damaging anti-pattern. It violates Errors P2 (Fail Loudly), P3 (Fail Specifically), P1 (Errors are Data), P4 (Preserve Context), P9 (Errors are Logged), P10 (Errors Don't Lie) — six of ten error principles in one catch block.
The downstream effects:
- Operators cannot debug (no log, no context).
- Users see wrong errors (404 for a 500).
- Security failures hide (authz error becomes "not found").
- The system appears healthy when it is not (no metrics, no logs).
The fix is always the same: **never swallow an error**. Log it, wrap it, rethrow it, or return it as a typed value. Never `return null` for "something went wrong."