Code Quality Guard

Code Quality Guard

Enforce simplicity-first coding principles. Prevent over-engineering. Write code that a tired developer at 2am can understand.

Core Principles

1. Surgical Changes Only

Touch only what needs to change. Do not refactor unrelated code in the same commit.

User asks: "Fix the login bug"

❌ WRONG: Rewrite the entire auth module, rename 15 files, restructure folders
✅ RIGHT: Change the 3 lines causing the bug, add a test, done

2. Simplicity Over Cleverness

Prefer obvious code over clever code. Every abstraction must earn its keep.

# ❌ Over-engineered
class AbstractFactoryProvider(metaclass=Singleton):
    def __init__(self):
        self._registry = {}
    def register(self, name, cls):
        self._registry[name] = cls
    def create(self, name, **kwargs):
        return self._registry[name](**kwargs)

# ✅ Simple
def create_user(data):
    return User(name=data["name"], email=data["email"])

// ❌ Over-engineered
const result = items
  .filter(Boolean)
  .map(x => x.value)
  .reduce((acc, v) => ({ ...acc, [v.key]: v.data }), {});

// ✅ Simple
const result: Record<string, Data> = {};
for (const item of items) {
  if (item) result[item.key] = item.data;
}

3. No Premature Abstraction

Do not create abstractions until you have 3+ concrete use cases.

First time: Write it inline
Second time: Note the duplication
Third time: NOW extract a shared function

❌ WRONG: Creating a generic "BaseService" class when you have one service
✅ RIGHT: Write the one service directly, extract later when pattern emerges

4. YAGNI (You Aren't Gonna Need It)

Do not add features, parameters, or flexibility for hypothetical future needs.

# ❌ "We might need pagination someday"
def get_users(page=None, per_page=None, sort_by=None, filters=None,
              include_deleted=False, soft_delete=True):
    ...

# ✅ Build what's needed now
def get_users():
    return db.query("SELECT * FROM users WHERE deleted = false")

Decision Tree

When writing or reviewing code, follow this order:

1. Does it solve the actual problem?
   NO → Stop. Understand the requirement first.

2. Is this the simplest solution that works?
   NO → Simplify. Remove abstractions, reduce indirection.

3. Does it touch files outside the scope of the task?
   YES → Revert unrelated changes.

4. Are there unused parameters, flags, or config options?
   YES → Remove them. YAGNI.

5. Would a junior developer understand this in 10 seconds?
   NO → Add clarity: better names, simpler structure.

6. Is there a design pattern being used?
   → Ask: "Does this pattern solve a real problem HERE, or am I pattern-matching from a blog post?"

Anti-Patterns to Flag

Factory of Factories

❌ If you see: AbstractFactory, FactoryFactory, ProviderFactory
✅ Replace with: A plain function that returns the thing

Dependency Injection for Everything

❌ Constructor with 10 injected dependencies for a simple controller
✅ Import what you need directly, inject only for testing seams

Configuration Explosion

❌ 50 config options where 5 would suffice
✅ Hard-code reasonable defaults, expose only what varies per environment

Deep Inheritance Hierarchies

❌ BaseService → CrudService → UserCrudService → AdminUserCrudService
✅ Composition: UserService has-a Validator, has-a Repository

Enterprise Naming

❌ IUserRepositoryFactoryImpl
✅ UserStore
❌ AbstractSingletonProxyFactoryBean
✅ create_proxy()

Review Checklist

When reviewing code, check:

• Does each function do exactly one thing?
• Are variable names descriptive without being verbose?
• Is there any code that exists "just in case"?
• Can you delete any files or classes without breaking anything?
• Is the solution proportional to the problem? (Don't use a cannon to kill a mosquito)
• Would you be comfortable explaining this code to a new team member?

Naming Rules

Functions: verb + noun → get_user(), calculate_total(), send_email()
Booleans: is/has/can/should → is_active, has_permission, can_edit
Collections: plural → users, error_list, config_items
Constants: UPPER_SNAKE → MAX_RETRIES, DEFAULT_TIMEOUT

❌ single_letter: i, j, k (only in short loops < 5 lines)
❌ vague: data, result, temp, thing, stuff, manager, handler
❌ hungarian: strName, intCount, bIsActive

The 10-Second Rule

If you cannot understand a function within 10 seconds of reading it, it needs simplification:

1. Extract complex conditions into named variables
2. Break long functions into smaller ones
3. Remove nesting (early returns > nested if-else)
4. Use guard clauses

# ❌ Hard to scan
def process(order):
    if order:
        if order.items:
            if order.customer.verified:
                return charge(order)
            else:
                raise Error("not verified")
        else:
            raise Error("empty")
    else:
        raise Error("no order")

# ✅ Easy to scan
def process(order):
    if not order:
        raise Error("no order")
    if not order.items:
        raise Error("empty")
    if not order.customer.verified:
        raise Error("not verified")
    return charge(order)

When NOT to Simplify

Some complexity is necessary. Do NOT simplify:

• Security-critical code (cryptography, auth tokens)
• Race condition handling (locks, atomics)
• Performance-critical hot paths (after profiling)
• Domain-specific logic that mirrors complex real-world rules

In these cases, add a comment explaining WHY the complexity exists.