beginnerintermediateadvanced

Python · Code Quality and Tooling

Engineering practices

Concepts: 3

The human-facing half of code quality — code review, clear naming, type annotations, documentation, small cohesive functions, consistent structure, avoiding premature abstraction, and refactoring/technical debt.

This section

Python Tutorial: Ruff - A Fast Linter & Formatter to Replace Multiple Tools and Improve Code QualityCorey Schafer

Readable and maintainable code

Making code fast to review and change — through naming, documentation, function design, and disciplined refactoring.

Code review, clear naming, type annotations, and documentation

coreintermediate

Code review is a second person reading code before it merges, catching what the author could not see. Clear naming, type annotations, and documentation make that review — and every later reading — fast, not a slow reverse-engineering act.

Think of it as

Code is read far more often than it is written — a reviewer, a future maintainer, or the same author six months later, all reading without the context that was in the author's head while writing. Naming, types, and docs are that missing context, written down once instead of re-derived every time.

python

def calculate_shipping_cost(order: Order) -> Decimal:
    """Return shipping cost, free above the $50 threshold.

    Raises ValueError if order.items is empty.
    """
    ...

What we're doing: Contrast a vaguely-named, undocumented function with one carrying clear naming, a type annotation, and a docstring that explains the non-obvious rule.

naming_and_docs.pypython

def calc(o):
    if o.total >= 50:
        return 0
    return 5.99


def calculate_shipping_cost(order: Order) -> Decimal:
    """Return shipping cost, free above the $50 threshold."""
    return Decimal("0") if order.total >= 50 else Decimal("5.99")

1: calc(o) tells a reader nothing — what does it calculate? What is o? A reviewer has to read the whole body to find out.
7: The name alone tells a reader what this does; -> Decimal tells them what comes back, checked by a type checker, not just claimed in a comment.
8: The docstring states the ONE fact not obvious from the code itself (the $50 threshold rule) — it does not restate what the code already says plainly.

Why this works: Both functions do the same thing — but a reviewer (or a future reader) understands the second one in seconds, from its name and one-line docstring alone, without needing to trace through the logic first.

Writing a docstring that just restates the code

Wrong

python

def get_user(user_id: int) -> User:
    """Get a user.

    Args:
        user_id: the user id
    Returns:
        the user
    """
    return db.query(User).get(user_id)

Better

python

def get_user(user_id: int) -> User:
    """Fetch a user by id.

    Raises UserNotFoundError if no user with this id exists —
    callers should not assume a User is always returned.
    """
    return db.query(User).get(user_id)

What you see: The docstring takes time to write and read but tells the reader nothing they could not already see from the function signature.

Why: A docstring earns its keep by stating something the signature and code do NOT already make obvious — a raised exception, a non-obvious edge case, a caller contract. "Get a user" adds nothing over the name get_user already saying exactly that.

The same function, unreadable vs. self-explaining

calc(o)

+Vague name — what does it calculate?
+No type hints — o could be anything
+No docstring — the $50 rule is hidden in the body

calculate_shipping_cost(order: Order) -> Decimal

−Name states exactly what it does
−-> Decimal is checked by a type checker, not just claimed
−Docstring states the one non-obvious rule, nothing else

Naming — vague vs. clear

Naming — vague vs. clear
Vague	Clear
n, temp, data	retry_count, parsed_config, active_users
def process(x):	def calculate_shipping_cost(order: Order) -> Decimal:
# increment counter	# retry only on 5xx -- 4xx means the request itself is invalid

Together

python

def calculate_shipping_cost(order: Order) -> Decimal:
    """Return shipping cost, free above the $50 threshold.

    Raises ValueError if order.items is empty.
    """
    if not order.items:
        raise ValueError("cannot calculate shipping for an empty order")
    return Decimal("0") if order.total >= 50 else Decimal("5.99")

Remember: Code review catches what a solo author cannot see; naming, types, and docstrings that explain WHY make code fast to read.

Small cohesive functions, project structure, and premature abstraction

coreintermediate

A small, cohesive function does one job and is named for it. A consistent project structure means every developer can guess where something lives. Premature abstraction generalizes before a second real use case exists.

Think of it as

A function that does five things is a Swiss Army knife pretending to be one tool — hard to test in isolation, hard to name honestly. Premature abstraction is buying a toolbox before knowing which tools you need; a second real, concrete case is what tells you the actual shape of the abstraction.

python

def validate_order(order): ...
def charge_order(order, gateway): ...
def send_confirmation(order): ...

def process_order(order, gateway):
    validate_order(order)
    charge_order(order, gateway)
    send_confirmation(order)

What we're doing: Refactor one function doing three jobs into three cohesive ones, and show each is independently testable.

cohesive_functions.pypython

def validate_order(order):
    if not order["items"]:
        raise ValueError("order must have at least one item")


def calculate_total(order):
    return sum(item["price"] for item in order["items"])


def process_order(order):
    validate_order(order)
    return calculate_total(order)


order = {"items": [{"price": 10}, {"price": 20}]}
print(process_order(order))

empty_order = {"items": []}
try:
    process_order(empty_order)
except ValueError as e:
    print("caught:", e)

1: validate_order does exactly one job — checking the order is valid — and can be tested with just a dict, no payment gateway needed.
6: calculate_total does exactly one job — summing prices — testable completely independently of validation.
10: process_order composes the two single-purpose functions — the orchestration itself is now trivially readable.

Output

30
caught: order must have at least one item

Why this works: Each function is small enough to understand at a glance and test with a one-line call — validate_order needs no calculate_total mock to test, and vice versa. process_order itself became almost self-documenting, since it just names the steps in order.

Building a generic "config-driven" abstraction before a second use case exists

Wrong

python

class GenericProcessor:
    def __init__(self, config, validators, transformers, handlers):
        # generalized for hypothetical future processors that don't exist yet
        ...
    # 200 lines of generic machinery, for exactly ONE real caller

Better

python

def process_order(order):
    validate_order(order)
    return calculate_total(order)
# extract a shared abstraction ONLY once a second real, concrete case appears

What you see: A large, flexible-looking class or framework exists for a single real caller — every "customization point" is speculative, untested by real variety, and often wrong once a genuine second use case finally appears.

Why: An abstraction's shape should come from at least two real, concrete cases — building it from one case (or zero) means guessing, and the guess is usually wrong in ways that only show up once a second real case reveals what actually varies.

One function, three jobs — vs. three cohesive ones

process_order() does everything

+Validates, charges, emails, logs — one function
+Any change risks breaking an unrelated concern
+Testing needs mocking payment AND email AND logging

validate_order(), charge_order(), ...

−Each function has exactly one reason to change
−Each is testable in isolation, one mock at a time
−process_order() just names the steps, in order

One function, several responsibilities — split

One function, several responsibilities — split
Before	After
process_order() validates, charges, emails, logs	validate_order(), charge_order(), send_confirmation(), log_order()
Each change risks breaking an unrelated concern	Each function has one reason to change
Testing requires mocking payment AND email AND logging	Each function tests in isolation, one mock at a time

Together

python

def validate_order(order):
    if not order.items:
        raise ValueError("order must have at least one item")

def charge_order(order, payment_gateway):
    return payment_gateway.charge(order.total)

def process_order(order, payment_gateway):
    validate_order(order)
    return charge_order(order, payment_gateway)

Remember: A cohesive function has one reason to change; write a concrete case twice before extracting a shared abstraction.

Refactoring safely and technical debt management

coreintermediate

Refactoring changes HOW code is structured without changing WHAT it does — a passing test suite before and after proves that. Technical debt is a shortcut that trades short-term speed for a larger cost paid later.

Think of it as

Refactoring without tests is renovating a house while blindfolded — you cannot tell if you broke something until it collapses later. Tests are the light switch: change the structure, flip the switch (run the suite), and know immediately whether behavior actually held.

python

# 1. Write/confirm tests covering current behavior
# 2. Refactor internal structure
# 3. Run the SAME tests -- unchanged behavior means they still pass
pytest test_module.py

What we're doing: Refactor a function's internal implementation while keeping a test suite green throughout, proving behavior did not change.

safe_refactor.pypython

def calculate_total_v1(items):
    total = 0
    for item in items:
        total = total + item["price"]
    return total


def calculate_total_v2(items):
    # refactored: same behavior, different (more idiomatic) implementation
    return sum(item["price"] for item in items)


items = [{"price": 10}, {"price": 20}, {"price": 30}]
print("v1:", calculate_total_v1(items))
print("v2:", calculate_total_v2(items))
print("same result:", calculate_total_v1(items) == calculate_total_v2(items))

1: The original implementation — a manual loop accumulating a running total.
8: The refactored implementation — sum() with a generator expression does the same job more idiomatically.
16: Comparing both confirms the refactor changed HOW the total is computed, not WHAT the result is — the actual definition of a safe refactor.

Output

v1: 60
v2: 60
same result: True

Why this works: Both implementations produce identical output for the same input — that equality is the concrete proof a refactor preserved behavior. In a real codebase, an existing test suite (not a manual side-by-side comparison) is what provides this same guarantee automatically, on every change.

Refactoring and adding new behavior in the same change

Wrong

python

def calculate_total(items):
    # "while I'm refactoring this, might as well add a discount too"
    return sum(item["price"] for item in items) * 0.9   # NEW behavior mixed in

Better

python

# Refactor commit: same behavior, cleaner code
def calculate_total(items):
    return sum(item["price"] for item in items)

# SEPARATE commit: new discount behavior
def calculate_total(items):
    return sum(item["price"] for item in items) * 0.9

What you see: A refactor introduces a real behavior change, but it is buried inside a commit labeled "refactor," making the change hard to review, hard to bisect if it causes a bug, and hard to revert independently.

Why: Mixing structural change with behavioral change defeats the entire point of calling something a refactor — a reviewer (and a test suite) can no longer distinguish "this is provably the same" from "this is new and needs its own scrutiny."

A safe refactor, one confirmable step at a time

Confirm coverage

tests exist and pass against current behavior, before any change

Change structure

extract, rename, or simplify — one small step, no new behavior mixed in

Run the SAME tests

green means behavior held; a single giant rewrite defers this too long

Repeat, or stop

each step is independently confirmable — nothing is "done" until tests prove it

Deliberate vs. accidental technical debt

Deliberate vs. accidental technical debt
Kind	Example	Risk if untracked
Deliberate	"ship the simple version now, generalize after launch" — a real TODO	Low, if actually revisited
Accidental	a rushed fix under deadline pressure, no comment explaining it	High — nobody remembers why it exists

Together

python

# TODO(2026-09-01): this hardcodes USD -- generalize once
# a second currency customer actually signs (tracked: JIRA-1234)
def calculate_total(items):
    return sum(item.price_usd for item in items)

Remember: A refactor changes structure, never behavior — a passing test suite before and after is the proof.