The code review comment that changed how I write functions

A four-word comment exposed a gap between naming functions and understanding what they’re for.

How a single margin note rewired the way I think about function scope

The comment was four words: “What does this actually do?”

I’d written a function called process_user_data. It was 87 lines long. It validated the input, fetched a record from the database, applied a transformation, logged the result, and returned a status code. I thought it was tidy. I'd even added a docstring.

My tech lead left that comment on line 1.

I read it three times, trying to decide if it was a compliment. It wasn’t. He was pointing at the name and telling me, without saying it directly, that I had written a function that did five things and called it one name. Not because I was lazy. Because I hadn’t yet learned to feel the difference between naming what code does and naming what code is for.

That distinction sounds trivial. It isn’t.

Photo by Mimi Thian on Unsplash

The function that does too much always starts the same way

It starts with good intentions. You need to handle a user record — fetch it, validate it, update a field, log the change. Putting it in one place feels like good design. DRY principle. Single responsibility, sort of.

The problem is that “single responsibility” is easy to misread as “single topic.” User data is a single topic. But validating, fetching, transforming, and logging are four responsibilities wearing the same coat.

Here’s roughly what I had:

def process_user_data(user_id: str, payload: dict) -> dict:
    # validate
    if not payload.get("email"):
        raise ValueError("Missing email")

    # fetch
    user = db.get(user_id)
    if not user:
        raise NotFoundError(f"User {user_id} not found")

    # transform
    user["email"] = payload["email"].lower().strip()
    user["updated_at"] = datetime.utcnow().isoformat()

    # persist
    db.save(user)

    # log
    logger.info(f"Updated user {user_id}")

    return {"status": "ok", "user_id": user_id}

It works. Tests pass. Code review approved it, technically — except for that comment.

What the comment forced me to ask: if this function fails, which part failed? If I want to reuse the transformation logic elsewhere, can I? If I want to skip the logging in a test, how?

The answers were: I don’t know, no, and I can’t.

What I understood differently

The rewrite wasn’t about splitting things into more files. It was about separating what can fail independently.

Validation can fail without touching the database. The transformation has no reason to know about persistence. Logging has no reason to know about either. When you entangle them, every caller takes on all the failure modes at once — even the ones that don’t apply to their context.

def validate_email_payload(payload: dict) -> None:
    if not payload.get("email"):
        raise ValueError("Missing email")

def apply_email_update(user: dict, payload: dict) -> dict:
    return {
        **user,
        "email": payload["email"].lower().strip(),
        "updated_at": datetime.utcnow().isoformat(),
    }

def update_user_email(user_id: str, payload: dict) -> dict:
    validate_email_payload(payload)
    user = db.get(user_id)
    if not user:
        raise NotFoundError(f"User {user_id} not found")
    updated = apply_email_update(user, payload)
    db.save(updated)
    logger.info(f"Updated email for user {user_id}")
    return {"status": "ok", "user_id": user_id}

update_user_email is now a coordinator. It still does five things — but it delegates rather than implementing each step inline. The distinction matters because now apply_email_update is pure. You can test it without a database. You can call it in a bulk migration without triggering logging. You can read the name and know exactly what it does.

The coordinator function is still allowed to be complex — it just shouldn’t be doing the work itself.

The thing that actually changed

After that review, I started asking a different question when naming a function. Not “what does this code do?” but “what would break if this function didn’t exist?”

If the answer is “a lot of things, for different reasons,” the function is doing too much. If the answer is precisely scoped — “the email field wouldn’t get normalized” — the function has found its size.

This shifted something concrete in how I write code day-to-day. I now notice the moment I type a comment inside a function body. A comment that labels a section — # validate, # fetch, # transform — is almost always a sign that the section wants to be a function. The comment is the function name, waiting to be extracted.

This doesn’t mean every function should be three lines. Some coordination logic is genuinely complex and belongs together. But the complexity should come from the coordination, not from the implementation leaking through.

The harder question I still don’t have a clean answer to: where does the coordinator stop? At some level, main() coordinates everything. Service classes coordinate their methods. At what granularity does the model break down — and is that granularity the same in a 10-person team as in a solo project where you’re the only reader?

I’ve landed somewhere provisional on that. But I’d be lying if I called it settled.