What Makes Code Good

2026-01-29

I've written the coding standards at half a dozen companies by now. I generally don't care about where you put the curly braces or if else's are cuddled, I'm happy to let an opinionated formatting tools decide that for me.

For every rule, I should be able to explain why it's valid. If my explanation is weak, so is my rule.

Nothing makes a team of engineers shudder like a 1 hour meeting invite on "code quality". You hired me for my good judgement, let me use it! And that's true: good judgement is necessary. Code quality on a proof of concept that won't be touched again after a week is different from code quality on a major update to a product with thousands of customers.

But code quality shouldn't feel like an extra tax we pay for no reason, or to earn some unquantifiable "production ready" badge. It should save us time. If that quick hack prototype suddenly gives the wrong answers, how hard will it be to fix? If it's vibe-coded from the start, do you have time to start all over again?

That's the test. Keep cognitive load low.

Is the code self-explanatory? Is there just enough documentation that I can quickly make sense of what's lurking in the AWS account? But not so much documentation that it's likely out of date or overly generic? Then we're good.

The best engineer I know told me this is how he reviews code: what would I have done?

It doesn't have to be implemented the same way I'd code it. But what are the edge cases I'd be thinking about? What are the problems I'd want to get ahead of? How does this implementation approach them? Did it miss any?

You're writing code for other humans to read. Machines need to be able to run it and they should give the right answer when they do. At some point they won't (we call that a bug), or the right answer will change, and a human will need to fix your code.

If an experienced software engineer familiar with the language saw this code, would they be surprised? If yes, the code needs work.

Sticking to standards and conventions means fewer surprises. If you're writing Python, follow PEP8 and enforce it with black or ruff. Ruby should pass RuboCop. Java has Google's style guide. Whatever your language, there's probably an accepted standard. Why reinvent the wheel?

Since autocomplete appeared in our IDEs it's been possible for the machine to suggest the wrong answer. Coding agents can create thousands of lines of suggested code. It's your name on the pull request, your name in the commit log. "Claude wrote that" isn't an acceptable excuse.

Your job is to deliver code you have proven to work.

Intent should be clear from code. Every line of comment needs to be as correct and bug-free as every line of code, except you can't write tests for comments. (Ok, maybe you could get an LLM to do it but no way that's going in my build pipeline.)

# allowed to place an order?
result = user.age >= 18 and user.verified and not user.restricted

if result:
    process_order()

Better:

can_purchase = user.age >= 18 and user.verified and not user.restricted
if can_purchase:
    process_order()

The variable name does the job the comment was doing. There's one less line that can drift out of sync, one less line to maintain.

I have to figure out why it's disabled. Git history will keep anything that we want to refer back to later. Commented out code is extra noise I have to wade through, and it's 4am and we're under pressure, remember?

#max_results = 120
max_results = 20

perform_query(max_results)

This gives me nightmares. Is that a typo? Was that some debugging change that was accidentally merged? Can I uncomment it?

Delete it. If you need it back, that's what version control is for.

When I read code I'm executing it in my head. The more context and state I need to keep track of, the more confused my little wet human brain gets.

If you can return early, and it makes the code easier to read, do that.

if user.age >= 18:
    if user.verified:
        if not user.restricted:
            return process_order()
        else:
            return RestrictedResponse()
    else:
        return NotVerifiedResponse()
return UnderageResponse()

Better:

if user.age < 18:
    return UnderageResponse()

if not user.verified:
    return NotVerifiedResponse()

if user.restricted:
    return RestrictedResponse()

return process_order()

I know this looks contrived, but it's easy for complex logic to accidentally accrue like sediment at the bottom of a murky river. The nested version forces you to mentally track which branch you're in. The flat version reads top to bottom: check, bail, check, bail, do the thing.

Similar to early returns, when I read a try block that's extra state I need to track in my head. What exceptions will be caught at the end of this chunk of code? What might be thrown by any line inside? If there are more than 5 or 6 lines, it gets hard to determine how appropriate the except code will be.

If you really need to catch exceptions over quite a lot of code, wrap it in a function. It'll make testing easier too.

try:
    user = fetch_user(user_id)
    orders = fetch_orders(user)
    validated = validate_order_items(orders)
    inventory = check_inventory(validated)
    total = calculate_total(validated, inventory)
    charge_customer(user, total)
    send_confirmation(user, orders)
except Exception as e:
    log.error(f"Order failed: {e}")
    return ErrorResponse()

Better:

try:
    user = fetch_user(user_id)
except UserNotFoundError:
    return UserNotFoundResponse()

try:
    orders = fetch_orders(user)
except OrderFetchError:
    return OrderErrorResponse()

validated = validate_order_items(orders)
inventory = check_inventory(validated)
total = calculate_total(validated, inventory)

try:
    charge_customer(user, total)
except PaymentError as e:
    log.error(f"Payment failed for user {user_id}: {e}")
    return PaymentFailedResponse()

send_confirmation(user, orders)

In Python, bare except: or except Exception: blocks are particularly nasty since they swallow errors you didn't anticipate, including things like KeyboardInterrupt or bugs in your code that should have crashed loudly.

If you can't see the whole function on your screen at once, it's probably doing too much. I need to keep too much state in my head at once to fully understand every path through it.

Sometimes it's hard to avoid, particularly for a pipeline of actions that just need to execute sequentially. But if you find yourself scrolling, that's often a sign you're missing some higher abstraction.

A function that mixes high-level orchestration (process_order()) with low-level details (cursor.execute("SELECT...")) is hard to follow. Keep each function at roughly one level of abstraction.

def process_order(order):
    # High-level: validate the order
    if not order.items:
        return InvalidOrderResponse()
    
    # Suddenly low-level: raw SQL
    conn = psycopg2.connect(host="localhost", dbname="orders")
    cursor = conn.cursor()
    cursor.execute(
        "INSERT INTO orders (user_id, total) VALUES (%s, %s)",
        (order.user_id, order.total)
    )
    conn.commit()
    
    # Back to high-level
    send_confirmation_email(order)

Better:

def process_order(order):
    if not order.items:
        return InvalidOrderResponse()
    
    save_order(order)
    send_confirmation_email(order)

The first example hurts my head and it's hard to even explain why. You've put mustard on your ice cream and I don't know if you really meant to do that.

The second example is easier to read and easier to test. You can verify the orchestration logic without setting up a database and test the persistence layer in isolation.

The other best engineer I know taught me this idea. We chase reusable code, not just for reuse but because it's generally a good sign. It's not too coupled to other components, it's got clear responsibilities and dependencies. It knows enough about enough and no more.

Your tests are the first reuse of your code. If you find yourself mocking huge amounts of your system, that's a smell. Your code is probably too tangled up with its dependencies.

Luckily, in languages like Python it's easy to inject dependencies:

def get_api_version(api_base: str, http_client=httpx) -> int:
    response = http_client.get(urljoin(api_base, "/api/version"))
    response.raise_for_status()
    return int(response.text)

In your tests you don't need to patch the httpx module, you can provide a dummy implementation. Callers outside tests don't need to care.

enlarged