Py Clean General

General Clean Code Principles

Critical Rules

G5: DRY (Don't Repeat Yourself)

Every piece of knowledge has one authoritative representation.

# Bad - duplication
tax_rate = 0.0825
ca_total = subtotal * 1.0825
ny_total = subtotal * 1.07

# Good - single source of truth
TAX_RATES = {"CA": 0.0825, "NY": 0.07}
def calculate_total(subtotal: float, state: str) -> float:
    return subtotal * (1 + TAX_RATES[state])

G16: No Obscured Intent

Don't be clever. Be clear.

# Bad - what does this do?
return (x & 0x0F) << 4 | (y & 0x0F)

# Good - obvious intent
return pack_coordinates(x, y)

G23: Prefer Polymorphism to If/Else

# Bad - will grow forever
def calculate_pay(employee):
    if employee.type == "SALARIED":
        return employee.salary
    elif employee.type == "HOURLY":
        return employee.hours * employee.rate
    elif employee.type == "COMMISSIONED":
        return employee.base + employee.commission

# Good - open/closed principle
class SalariedEmployee:
    def calculate_pay(self): return self.salary

class HourlyEmployee:
    def calculate_pay(self): return self.hours * self.rate

class CommissionedEmployee:
    def calculate_pay(self): return self.base + self.commission

G25: Replace Magic Numbers with Named Constants

# Bad
if elapsed_time > 86400:
    ...

# Good
SECONDS_PER_DAY = 86400
if elapsed_time > SECONDS_PER_DAY:
    ...

G30: Functions Should Do One Thing

If you can extract another function, your function does more than one thing.

G36: Law of Demeter (Avoid Train Wrecks)

# Bad - reaching through multiple objects
output_dir = context.options.scratch_dir.absolute_path

# Good - one dot
output_dir = context.get_scratch_dir()

Composition Over Inheritance

Inheritance couples a subclass to its parent's internals forever. Use it only for a genuine is-a relationship where the subclass is substitutable everywhere the parent is. For reuse, compose.

# Bad - inherits to borrow behaviour, and drags in everything else
class EmailNotifier(SMTPClient):
    def notify(self, user, message):
        self.send(user.email, message)   # now coupled to every SMTPClient method

# Good - holds what it needs
class EmailNotifier:
    def __init__(self, smtp: SMTPClient) -> None:
        self._smtp = smtp

    def notify(self, user: User, message: str) -> None:
        self._smtp.send(user.email, message)

The composed version can be tested with a fake SMTPClient, can swap transports, and exposes only notify. The inherited version exposes SMTP's whole surface as part of its own API.

Signals you inherited for the wrong reason: the subclass overrides a method to raise NotImplementedError, ignores parameters the parent requires, or the hierarchy is three deep and the behaviour lives in the middle layer.

# Bad - a square is not substitutable for a rectangle
class Square(Rectangle):
    def set_width(self, w): self._w = self._h = w   # breaks every caller of Rectangle

# Good - share the contract, not the implementation
class Shape(Protocol):
    def area(self) -> float: ...

Prefer a Protocol for the shared contract, and a plain attribute for the shared behaviour. Mixins are inheritance too — the same test applies. Where a subclass adds no behaviour, a dataclass field or a function argument is enough.

Enforcement Checklist

When reviewing AI-generated code, verify:

• No duplication (G5)
• Clear intent, no magic numbers (G16, G25)
• Polymorphism over conditionals (G23)
• Functions do one thing (G30)
• No Law of Demeter violations (G36)
• Boundary conditions handled (G3)
• Dead code removed (G9)
• Composition preferred over inheritance
• Module reads top-down, callers above callees