Code Review · Python

Python Code Review: Using Exceptions for Control Flow

Review Python code that uses broad try/except blocks for normal lookup logic and silently hides different failure cases.

Code Review Exercise

Review the following code and identify as many issues as you can. Think about correctness, readability, intent, and when exceptions should actually be used.

python
def get_user_email(users_by_id, user_id):
    try:
        return users_by_id[user_id]["email"]
    except:
        return None


def has_admin_role(user):
    try:
        return "admin" in user["roles"]
    except:
        return False

What concerns do you see before scrolling further?

Review Findings

Issue #1: Normal lookup logic is hidden inside exceptions

The code uses try/except for cases that may be normal and expected, such as a missing user, missing email, or missing roles field. That makes the happy path and failure cases less explicit.

Issue #2: The bare except catches too much

A bare except: catches almost everything. It can hide real programming mistakes, such as a misspelled variable, an unexpected data shape, or a bug inside a helper function.

Issue #3: Different failures are treated the same

A missing key, a None value, a wrong data type, and an actual programming bug all produce the same result. The caller cannot tell whether the data was absent or the code failed.

Issue #4: The intent is unclear

Returning None or False may be correct, but the code does not make the contract clear. A reviewer should ask whether missing data is expected, exceptional, or invalid input.

Why This Happens

Python makes exceptions easy to use, and there are cases where the EAFP style — easier to ask forgiveness than permission — is perfectly reasonable.

The issue is not that exceptions are bad. The issue is using broad exceptions to hide ordinary branching logic or to suppress failures without understanding what failed.

Better Version: Make Expected Cases Explicit

python
def get_user_email(users_by_id: dict[int, dict], user_id: int) -> str | None:
    user = users_by_id.get(user_id)
    if user is None:
        return None

    email = user.get("email")
    if email is None:
        return None

    return email


def has_admin_role(user: dict) -> bool:
    roles = user.get("roles", [])
    return "admin" in roles

This version makes the expected missing-data cases visible. A reader does not need to infer the behavior from a broad exception handler.

Better Version: Catch Specific Exceptions When Needed

python
def get_user_email(users_by_id: dict[int, dict], user_id: int) -> str | None:
    try:
        return users_by_id[user_id]["email"]
    except KeyError:
        return None

This version is still exception-based, but it catches the specific expected failure. It does not accidentally hide unrelated bugs.

What To Say In An Interview

A good review does not simply say, "Never use exceptions for control flow." That is too broad.

A stronger answer sounds like this:

I would avoid the bare except here because it hides too many failure modes. If missing data is expected, I would make that explicit with get() or catch only KeyError. If other exceptions happen, I probably want to see them instead of silently returning None or False.

Interview Follow-Ups

Are exceptions always bad for control flow in Python?

No. Python commonly uses exceptions in some idioms. The concern is broad or silent exception handling that hides expected and unexpected failures together.

What is better than a bare except?

Catch the specific exception you expect, such as KeyError, ValueError, or FileNotFoundError. Let unexpected exceptions surface.

When is dict.get() better?

dict.get() is often clearer when missing keys are normal and you want a default value or None.

What should a reviewer ask about this code?

Ask whether missing data is expected, whether returning None is part of the method contract, and whether unexpected bugs should be logged or allowed to fail.

Final Takeaway

Exceptions are useful for exceptional or clearly defined failure cases. But in code review, be suspicious when a broad exception handler quietly turns many different problems into the same default value.