Control Flow Anti-Patterns - What Not to Do and Why

Reading time: ~24 minutes | Level: Foundation → Engineering

What is wrong with this function?

def process_order(order, user, config):
    if order is not None:
        if user is not None:
            if user.is_active:
                if not user.is_banned:
                    if config.get("payments_enabled"):
                        found = False
                        for item in order.items:
                            if item.in_stock:
                                found = True
                                break
                        if found:
                            if order.total > 0:
                                if bool(order.status == "pending") == True:
                                    process(order)

Count the problems: deeply nested if blocks, a flag variable (found), bool(x) == True redundancy, magic string "pending", an if order is not None that could be a guard clause, and no else handling. If you can spot all of those in thirty seconds you are well-prepared for production code review. If some of them escaped you, this lesson will change how you read and write Python.

Control flow anti-patterns are not syntax errors. They pass all tests that check the happy path. They compile, they run, they often produce correct output for the cases that were anticipated. They reveal themselves when a requirement changes, when a new engineer tries to understand the code in six months, or when an edge case in production triggers the untested path that had no else clause.

This lesson is a synthesis of everything in this module, viewed through the lens of what goes wrong.

What You Will Learn

• Ten specific control flow anti-patterns with bad code, root cause analysis, and clean refactored solutions
• How to measure the impact of anti-patterns using cyclomatic complexity and cognitive complexity
• A code review checklist for control flow - what to look for in any pull request
• Six interview questions on anti-pattern recognition and remediation
• A Level 3 challenge: a 50-line function full of anti-patterns for you to refactor

Prerequisites

• All previous lessons in this module (Lessons 01–11)
• Python functions and scope (Module 3)
• Python exceptions (Module 5)
• Python enums (Module 4)

The Measurement Framework: What Anti-Patterns Cost

Before cataloguing anti-patterns, establish why they matter in measurable terms.

Cyclomatic complexity (McCabe, 1976) counts the number of linearly independent paths through a function. Every if, elif, for, while, except, case adds 1 to the count. A function with cyclomatic complexity above 10 is difficult to test fully. Above 20, it is considered unmaintainable by most style guides.

Cognitive complexity (SonarSource) is a more modern metric that weights nesting more heavily than cyclomatic complexity. Nested conditions cost more than flat ones because they require a reader to hold more context simultaneously. Every indentation level adds an extra weight to the next condition.

Anti-Pattern	Cyclomatic	Cognitive	Testing Cost
Arrow / Pyramid of Doom	low (+1/if)	HIGH (×nest)	HIGH
Flag variables	+1	medium	medium
Exception for flow ctrl	+1/except	medium	HIGH (infra)
Condition duplication	+1 each dup	medium	HIGH (drift)
Magic number conditions	none	medium	medium
Boolean parameter flag	+1 per flag	HIGH	HIGH
else after return	none	low	low (style)
Redundant conditions	none	low	low
Shadowed loop variable	none	medium	medium
while True without exit	+1 per loop	HIGH	HIGH (hang)

Anti-Pattern 1 - The Arrow / Pyramid of Doom

Deep nesting is the most visually obvious anti-pattern. Each nested if pushes the reader's mental context one level deeper. At depth four or more, a reader must simultaneously track every enclosing condition to understand what the innermost block means.

Bad Code

def submit_payment(user, card, amount, merchant):
    if user is not None:
        if user.is_verified:
            if card is not None:
                if card.is_active:
                    if not card.is_expired:
                        if amount > 0:
                            if merchant.accepts_payments:
                                charge(card, amount, merchant)
                                return "success"
                            else:
                                return "merchant_unavailable"
                        else:
                            return "invalid_amount"
                    else:
                        return "card_expired"
                else:
                    return "card_inactive"
            else:
                return "no_card"
        else:
            return "unverified_user"
    else:
        return "no_user"

Why it is problematic: The happy path (charge(...)) is buried at indentation level 8. There are 7 nested if blocks. A reader must scan downward to find the actual work being done, then scroll back up to reconstruct the seven conditions that must all be true for it to execute. Adding an eighth condition (say, if not user.is_rate_limited) requires restructuring the entire nesting tree. Cognitive complexity: 28 (far above the recommended maximum of 15).

Fix: Guard Clauses + Early Return

def submit_payment(user, card, amount, merchant):
    """
    Validate all preconditions as guard clauses at the top.
    The happy path is at the bottom with no indentation.
    """
    if user is None:
        return "no_user"
    if not user.is_verified:
        return "unverified_user"
    if card is None:
        return "no_card"
    if not card.is_active:
        return "card_inactive"
    if card.is_expired:
        return "card_expired"
    if amount <= 0:
        return "invalid_amount"
    if not merchant.accepts_payments:
        return "merchant_unavailable"

    # Happy path: all guards passed
    charge(card, amount, merchant)
    return "success"

Cognitive complexity: 7. Adding a new guard requires inserting two lines. No restructuring required.

:::tip The Guard Clause Rule Every nested condition of the form if valid: ... else: return error can be inverted to a guard: if not valid: return error. Repeat this inversion for every level of nesting and your function becomes flat. :::

Anti-Pattern 2 - Flag Variables

A flag variable is a boolean that is set inside a loop and checked after the loop. It is a symptom of translating the loop logic into explicit state management that Python already handles natively.

Bad Code

def find_admin(users):
    found = False
    admin_user = None
    for user in users:
        if user.role == "admin" and user.is_active:
            found = True
            admin_user = user
            break
    if found:
        return admin_user
    else:
        return None

Why it is problematic: found is a proxy for information already encoded in whether admin_user is None. The flag adds a variable that must be initialised, set, and checked - three places where a bug can appear instead of one. The for/else clause and next() with a default both handle this pattern natively in Python.

Fix 1: for/else clause

def find_admin(users):
    for user in users:
        if user.role == "admin" and user.is_active:
            return user
    return None

Fix 2: next() with a default

def find_admin(users):
    return next(
        (user for user in users if user.role == "admin" and user.is_active),
        None
    )

Fix 3: any() for boolean result only

def has_admin(users) -> bool:
    return any(user.role == "admin" and user.is_active for user in users)

:::note Flag Variables and any()/all() When the flag variable only records whether a condition was ever true (not which element satisfied it), any() is idiomatic. When you need the element itself, use next() with a generator expression and a default. Both avoid the flag entirely. :::

Anti-Pattern 3 - Using Exceptions for Normal Flow Control

Exceptions carry a semantic contract: they signal unexpected conditions. When you use try/except to handle expected, normal cases (like "key not in dictionary" during a standard lookup), you violate this contract and incur both performance overhead and conceptual confusion.

Bad Code

def get_user_timezone(config, user_id):
    try:
        return config["users"][user_id]["timezone"]
    except KeyError:
        return "UTC"


def process_items(items):
    i = 0
    while True:
        try:
            item = items[i]
            process(item)
            i += 1
        except IndexError:
            break   # using IndexError as the loop termination signal

Why it is problematic: KeyError during dictionary access is not exceptional - it is a normal case when data may or may not be present. Using exceptions for flow control makes try/except blocks a signal of "something can go wrong here" rather than "I'm testing whether data exists." It also has measurable performance overhead in CPython: exception raising is slower than a dictionary .get() call. The second example uses IndexError as a loop sentinel, which obscures the loop's intent entirely.

Fix: LBYL (Look Before You Leap) where appropriate

def get_user_timezone(config, user_id):
    # Use .get() with nested defaults - no exception needed
    return config.get("users", {}).get(user_id, {}).get("timezone", "UTC")


def process_items(items):
    # Standard for loop - StopIteration is handled internally, invisibly
    for item in items:
        process(item)

:::warning EAFP vs LBYL Python's official style guide prefers EAFP (Easier to Ask Forgiveness than Permission) in certain contexts - particularly around attribute access on objects whose type is not known. But EAFP should not be used as a justification for catching KeyError instead of using .get(), or AttributeError instead of hasattr(). Use exceptions for genuinely exceptional conditions: network failures, file system errors, unparseable external data. Use LBYL for predictable "does this exist?" checks on well-understood data structures. :::

Anti-Pattern 4 - Condition Duplication

Duplicate conditions - the same predicate checked in multiple places - signal that a boolean concept has not been named and extracted. When the condition changes, every duplicate must be updated in sync.

Bad Code

def render_dashboard(user):
    if user.subscription == "premium" and user.account_age_days > 30:
        show_analytics_widget()

    # ... 50 lines of other code ...

    if user.subscription == "premium" and user.account_age_days > 30:
        show_export_button()

    if user.subscription == "premium" and user.account_age_days > 30:
        show_advanced_filters()

Why it is problematic: The predicate user.subscription == "premium" and user.account_age_days > 30 is a business concept - "is this an established premium user?" - but it has no name in the code. If the requirement changes to 45 days instead of 30, you must find and update every duplicate. Missing one duplicate creates a behaviour inconsistency that is difficult to detect.

Fix: Extract to a named predicate

def is_established_premium_user(user) -> bool:
    """Returns True if the user is a premium subscriber with >30 days account age."""
    return user.subscription == "premium" and user.account_age_days > 30


def render_dashboard(user):
    if is_established_premium_user(user):
        show_analytics_widget()

    # ... 50 lines of other code ...

    if is_established_premium_user(user):
        show_export_button()

    if is_established_premium_user(user):
        show_advanced_filters()

Alternatively, evaluate once and reuse the result:

def render_dashboard(user):
    premium_established = is_established_premium_user(user)

    if premium_established:
        show_analytics_widget()

    # ...

    if premium_established:
        show_export_button()

    if premium_established:
        show_advanced_filters()

Anti-Pattern 5 - Magic Number Conditions

A magic number is a bare literal value in a condition whose meaning is not self-evident. It requires the reader to consult documentation, comments, or institutional knowledge to understand what the number represents.

Bad Code

def process_transaction(transaction):
    if transaction.status == 2:
        refund(transaction)
    elif transaction.status == 3:
        flag_for_review(transaction)
    elif transaction.status == 7:
        archive(transaction)


def check_risk(score):
    if score > 85:
        return "high_risk"
    elif score > 60:
        return "medium_risk"
    return "low_risk"

Why it is problematic: status == 2 means nothing to a reader who does not know the status code scheme. If status 2 is later reassigned to a different meaning, the condition silently becomes wrong. For the risk score, 85 and 60 have no labels - they are business thresholds that belong in a configuration or named constant.

Fix: Named constants or Enums

from enum import IntEnum

class TransactionStatus(IntEnum):
    PENDING = 1
    REFUNDED = 2
    FLAGGED = 3
    EXPIRED = 4
    CANCELLED = 5
    SETTLED = 6
    ARCHIVED = 7


def process_transaction(transaction):
    match transaction.status:
        case TransactionStatus.REFUNDED:
            refund(transaction)
        case TransactionStatus.FLAGGED:
            flag_for_review(transaction)
        case TransactionStatus.ARCHIVED:
            archive(transaction)
        case _:
            pass   # other statuses require no action


# Named thresholds for the risk model
HIGH_RISK_THRESHOLD = 85
MEDIUM_RISK_THRESHOLD = 60

def check_risk(score: int) -> str:
    if score > HIGH_RISK_THRESHOLD:
        return "high_risk"
    if score > MEDIUM_RISK_THRESHOLD:
        return "medium_risk"
    return "low_risk"

Anti-Pattern 6 - Boolean Parameter Flags

A boolean parameter that controls which of two fundamentally different execution paths a function takes is a sign that the function is doing two different things and should be two separate functions.

Bad Code

def process_data(data, validate=True, verbose=False, dry_run=False):
    if verbose:
        print(f"Processing {len(data)} records")

    if validate:
        if not is_valid(data):
            if verbose:
                print("Validation failed")
            return False

    results = []
    for item in data:
        if verbose:
            print(f"Processing item: {item}")
        result = transform(item)
        if not dry_run:
            save(result)
        results.append(result)

    if verbose:
        print(f"Done. {len(results)} items processed")
    return results

Why it is problematic: validate, verbose, and dry_run each create separate branches throughout the function. The function has 2³ = 8 distinct behaviour modes. Testing it requires combinations of all three flags. The reader must mentally simulate all three flags simultaneously to understand what any given execution path does.

Fix 1: Separate functions with clear names

def process_data(data: list) -> list:
    """Process and persist data. Raises ValueError on invalid input."""
    validate_data(data)    # raises on failure
    results = [transform(item) for item in data]
    for result in results:
        save(result)
    return results


def dry_run_process(data: list) -> list:
    """Process data without persisting - for testing and preview."""
    validate_data(data)
    return [transform(item) for item in data]


def validate_data(data: list) -> None:
    """Validate data, raising ValueError if invalid."""
    if not is_valid(data):
        raise ValueError(f"Data validation failed for {len(data)} records")

Fix 2: Strategy pattern for complex variation

from dataclasses import dataclass
from typing import Callable

@dataclass
class ProcessingConfig:
    save_fn: Callable       # real save or no-op
    log_fn: Callable        # real logger or no-op

PRODUCTION_CONFIG = ProcessingConfig(save_fn=save, log_fn=print)
DRY_RUN_CONFIG   = ProcessingConfig(save_fn=lambda r: None, log_fn=print)
SILENT_CONFIG    = ProcessingConfig(save_fn=save, log_fn=lambda *a: None)

def process_data(data: list, config: ProcessingConfig) -> list:
    results = [transform(item) for item in data]
    for result in results:
        config.save_fn(result)
    config.log_fn(f"Processed {len(results)} items")
    return results

:::danger The Boolean Flag Trap A function signature like def send_email(user, is_test=False) that has completely different behaviour for True vs False fails the single-responsibility principle. It is two functions sharing one name. The caller's intent - send_email(user, is_test=True) - is harder to read than send_test_email(user). Prefer two functions with descriptive names. :::

Anti-Pattern 7 - else After return or raise

When a function has already returned or raised, any code after that point is unconditionally unreachable. An else block after a return or raise creates unnecessary indentation without adding any logical value.

Bad Code

def get_discount(membership: str) -> float:
    if membership == "gold":
        return 0.20
    else:                           # unreachable after return - else is redundant
        if membership == "silver":
            return 0.10
        else:                       # nested redundant else
            return 0.05


def validate_age(age: int) -> None:
    if age < 0:
        raise ValueError(f"Age cannot be negative: {age}")
    else:                           # unreachable after raise
        if age > 150:
            raise ValueError(f"Age {age} exceeds plausible maximum")
        else:                       # nested redundant else
            print(f"Age {age} is valid")

Why it is problematic: The else blocks create additional indentation, pushing the code rightward for no logical reason. They mislead readers into thinking the else is guarding against something - but since the if block always returns or raises, the else is never a guard. This is purely visual noise that increases cognitive complexity without adding information.

Fix: Remove redundant else

def get_discount(membership: str) -> float:
    if membership == "gold":
        return 0.20
    if membership == "silver":
        return 0.10
    return 0.05


def validate_age(age: int) -> None:
    if age < 0:
        raise ValueError(f"Age cannot be negative: {age}")
    if age > 150:
        raise ValueError(f"Age {age} exceeds plausible maximum")
    print(f"Age {age} is valid")

Anti-Pattern 8 - Redundant Conditions

Redundant conditions use more code than necessary to express a boolean test. They are usually caused by misunderstanding Python's truthiness rules or overly defensive programming.

Bad Code

# Checking if something is True that is already True
if is_valid == True:               # redundant: is_valid is already a bool
    process()

if bool(flag) == True:             # double redundant: bool() + == True
    process()

# Checking container length explicitly
if len(items) > 0:                 # items is truthy iff non-empty
    process(items)

if len(errors) == 0:               # errors is falsy iff empty
    commit()

# Comparing to None using equality
if result == None:                 # should use 'is None'
    return default

if response != None:               # should use 'is not None'
    process(response)

Why it is problematic: if is_valid == True: is logically correct but violates Python idiom. It signals unfamiliarity with truthiness. len(items) > 0 is misleading - it implies there is something meaningful about zero versus a larger number, when the intent is just "is this non-empty?". == None comparisons can produce unexpected results with objects that override __eq__.

Fix: Idiomatic Python truthiness

# Bool directly
if is_valid:
    process()

# Non-empty container
if items:
    process(items)

# Empty container
if not errors:
    commit()

# None checks - always use 'is' / 'is not'
if result is None:
    return default

if response is not None:
    process(response)

:::note When to Use Explicit Length Check Use len(items) > 0 (or len(items) == 0) only when you genuinely care about length for a reason other than truthiness - for example, when you need to compare lengths (if len(a) > len(b)) or when the sequence has a custom __bool__ that you cannot rely on. For "is this collection non-empty?", use if items:. :::

Anti-Pattern 9 - Shadowing Loop Variables in Nested Loops

Reusing the same variable name (i, j, item, x) in nested loops is a subtle source of bugs. The inner loop variable shadows the outer one within its scope, and any code that references the variable after the inner loop exits may silently use the wrong value.

Bad Code

def compare_all_pairs(items):
    results = []
    for i, item in enumerate(items):
        for i, other in enumerate(items):    # BUG: outer 'i' is shadowed
            if item != other:
                results.append((i, i, item, other))  # both 'i' are the inner i
    return results


def find_duplicates(matrix):
    for item in matrix:                # outer 'item' is a row
        for item in item:              # BUG: inner 'item' shadows outer, also: for x in item!
            print(item)

Why it is problematic: After the inner loop exits, i holds the last value from the inner loop - not the outer loop's current value. Any code that references i after the inner for block will silently use the wrong index. Python does not warn about this because shadowing a variable is legal.

Fix: Use distinct, descriptive variable names

def compare_all_pairs(items):
    results = []
    for outer_idx, item in enumerate(items):
        for inner_idx, other in enumerate(items):
            if item != other:
                results.append((outer_idx, inner_idx, item, other))
    return results


def find_duplicates(matrix):
    for row in matrix:
        for cell in row:
            print(cell)

Anti-Pattern 10 - while True Without a Clear Exit

A while True loop with no clearly documented termination strategy is a latent hang risk. Even when a break exists, if the break condition is buried or reachable only through a complex chain of conditions, the loop's termination is non-obvious.

Bad Code

def read_until_empty(queue):
    while True:
        item = queue.get()
        if item:
            process(item)
        # BUG: what terminates this? queue.get() blocks forever if queue is empty
        # There is no break, no timeout, and no termination condition.


def retry_request(url):
    while True:
        try:
            response = http.get(url)
            if response.ok:
                return response.json()
            # BUG: if response is never ok, this loops forever
        except ConnectionError:
            pass   # silent retry, no limit, no backoff

Why it is problematic: A loop without a clear exit can hang a process, exhaust CPU, exhaust memory (if accumulating results), or prevent graceful shutdown. The while True pattern signals intent to loop forever, which is only appropriate for event loops in servers and daemons - and even then, it should have a shutdown signal handler.

Fix 1: Explicit loop condition

def read_until_empty(queue):
    while not queue.empty():
        item = queue.get_nowait()
        process(item)

Fix 2: Bounded retry with documented exit conditions

import time

def retry_request(url, max_attempts: int = 3, backoff_seconds: float = 1.0):
    """
    Retry HTTP GET with exponential backoff.
    Exits on success or after max_attempts failures.
    """
    for attempt in range(1, max_attempts + 1):
        try:
            response = http.get(url, timeout=10)
            if response.ok:
                return response.json()
            raise ValueError(f"Non-OK response: {response.status_code}")
        except (ConnectionError, ValueError) as exc:
            if attempt == max_attempts:
                raise RuntimeError(f"Failed after {max_attempts} attempts") from exc
            time.sleep(backoff_seconds * attempt)   # exponential backoff

Fix 3: Sentinel-based while loop (readable termination)

def interactive_session():
    """Process user commands until 'exit' is entered."""
    command = input("> ")
    while command != "exit":
        handle(command)
        command = input("> ")
    print("Session ended.")

Code Review Checklist for Control Flow

Use this checklist when reviewing any function that contains conditional logic or loops.

Nesting and Structure

• Is nesting deeper than 3 levels? If so, apply guard clauses.
• Could a nested if/else be flattened with an early return?
• Is the happy path visible without scrolling past guard clauses?

Variables

• Are any boolean flag variables present? Replace with for/else or any().
• Are loop variable names reused in nested loops?
• Are magic number literals present in conditions? Replace with named constants.

Completeness

• Does every if/elif chain have an else or a documented reason for omitting it?
• Does every loop have a clearly reachable termination condition?
• Does every function return a defined value for all inputs?

Boolean Logic

• Is if x == True: or if bool(x) == True: present? Replace with if x:.
• Is if len(x) > 0: present for non-empty checks? Replace with if x:.
• Is == None or != None used? Replace with is None / is not None.

Duplication

• Is the same condition evaluated in more than one place? Extract to a function.
• Does the same if/elif structure appear in multiple functions? Extract to a shared predicate.

Function Design

• Do boolean parameters control fundamentally different behaviour? Split into separate functions.
• Is else: used after a return or raise? Remove it.
• Are exceptions used to handle expected, non-exceptional cases? Use .get(), hasattr(), or LBYL.

Naming

• Do condition results have names that explain what they mean?
• Are status codes, role strings, or numeric thresholds documented with constants or enums?

How Anti-Patterns Relate to Cyclomatic Complexity

# Cyclomatic complexity = 1 (base) + 1 per decision point
def classify(score):           # complexity: 1
    if score >= 90:            # +1 → 2
        return "A"
    elif score >= 80:          # +1 → 3
        return "B"
    elif score >= 70:          # +1 → 4
        return "C"
    else:
        return "F"
    # Final complexity: 4 - fully testable with 4 test cases

def process_order_antipattern(order, user, config):  # complexity: 1
    if order is not None:          # +1 → 2
        if user is not None:       # +1 → 3
            if user.is_active:     # +1 → 4
                if not user.is_banned:   # +1 → 5
                    if config.get("enabled"):  # +1 → 6
                        found = False
                        for item in order.items:  # +1 → 7
                            if item.in_stock:     # +1 → 8
                                found = True
                                break
                        if found:       # +1 → 9
                            if order.total > 0:   # +1 → 10
                                process(order)
    # Final complexity: 10 - but many branches have no else, so actual paths > 10

Flattening the first function's pyramid of doom reduces cyclomatic complexity from 10 to 8 (each guard clause is still a decision point) but dramatically reduces cognitive complexity because the nesting weight is eliminated.

Interview Questions

Q1: What is the flag variable anti-pattern, and what are three Pythonic alternatives?

Answer: A flag variable is a boolean that is initialised before a loop, set inside the loop when a condition is found, and then checked after the loop to determine what happened. It anti-pattern because it adds manual state management that Python handles natively. Three alternatives: (1) for/else - the else block runs only if no break occurred, replacing found = False ... if not found: ...; (2) next() with a default - next((x for x in items if condition(x)), None) returns the first matching element or a default, eliminating the flag entirely; (3) Extract to a function - return from inside the loop on first match, returning None at the end if no match found. No flag variable needed.

Q2: When is using a boolean parameter (def func(data, dry_run=False)) a design problem?

Answer: A boolean parameter is a design problem when it causes the function to have fundamentally different behaviour paths - when True and False mean "do completely different things" rather than "do the same thing with or without a minor variation." The test is: if you drew a flowchart of the function with both parameter values, would the two paths share most of their logic, or would they diverge early and never converge? If they diverge early, you have two functions hiding inside one. The fix is to split into two named functions with descriptive names (process_data() and dry_run_process()), or to use a strategy/configuration object that makes the variation explicit and composable. The boolean flag hides the fact that you are making a design choice.

Q3: Why is else after return or raise considered an anti-pattern, and is it always wrong?

Answer: else after return or raise is redundant because the branch containing the return or raise has already exited the function - the else block would execute regardless of its presence. The anti-pattern is not a bug, it is unnecessary indentation that makes readers mentally track a guard that does not exist. It is considered wrong because it adds cognitive overhead (readers see if/else and expect mutual exclusivity, but the if already exits) and adds one indentation level to all code that follows. It is almost always better to remove the else and unindent the subsequent code. The only case where keeping it has merit is when the two branches are very short and keeping if/else makes the symmetry visually obvious - but this is a style judgement, not a technical requirement.

Q4: What is condition duplication and how does it cause bugs over time?

Answer: Condition duplication occurs when the same boolean predicate is evaluated in multiple places in the code rather than being extracted to a named function evaluated once. It causes drift bugs: when a requirement changes (e.g., "premium users now need 45 days account age instead of 30"), every duplicate must be updated. If even one is missed, different parts of the system behave inconsistently for the same input - premium users may see the analytics widget but not the export button, depending on which condition was updated. The fix is to extract the condition to a named predicate function (is_established_premium_user(user)) which becomes the single source of truth. Changing the rule in one place changes behaviour everywhere it is used.

Q5: When is using try/except for flow control appropriate, and when is it an anti-pattern?

Answer: Using try/except for flow control is appropriate (and idiomatic Python - EAFP) when: the failure case is genuinely unexpected, the cost of checking first is higher than catching the exception (e.g., checking file existence before reading can race with deletion), or the "ask for forgiveness" pattern is significantly cleaner (try: return d[key] except KeyError: return default). It becomes an anti-pattern when: (1) the exception is expected and common (catching KeyError in every .get()-equivalent lookup); (2) the exception is used as a loop termination signal instead of a proper loop condition; (3) the exception carries no information useful to the handler (except Exception: pass). Rule of thumb: if you are catching a specific exception type that represents a normal, expected case and a simpler non-exception test exists (.get(), hasattr(), in), use the non-exception test.

Q6: What is the "pyramid of doom" and what metric captures its cost objectively?

Answer: The pyramid of doom (or "arrow anti-pattern") is a pattern of deeply nested if blocks that indent progressively to the right, creating a visual shape resembling an arrow or pyramid. Objectively, its cost is captured by cognitive complexity - a metric that weights each nesting level multiplicatively rather than additively. A condition at nesting depth 4 contributes 4× to the cognitive complexity score compared to 1× for a flat condition at depth 1. This reflects the empirical observation that a reader must hold all enclosing conditions in working memory to understand the innermost block. Cyclomatic complexity captures the number of paths but not the nesting weight. The fix is guard clauses (early returns) that invert each nested condition and return immediately, reducing nesting to depth 1 for all conditions and the happy path.

Quick Reference Cheatsheet

Anti-Pattern	Symptom	Fix
Arrow of Doom	Nesting > 3 levels	Guard clauses + early return
Flag variable	found = False; for...; if found:	for/else, any(), next()
Exception for flow	try: d[k] except KeyError:	.get(), in, hasattr()
Condition duplication	Same predicate in 3 places	Extract to named predicate function
Magic numbers	if status == 2:	IntEnum, named constants
Boolean param flag	def f(x, validate=True)	Two separate named functions
else after return	if x: return a; else: return b	Remove else:, unindent
Redundant condition	if bool(x) == True:	if x:
Shadowed loop var	for i in ...: for i in ...:	Distinct descriptive names
while True no exit	Loop with no termination	Explicit condition or bounded retry

Graded Practice Challenges

Level 1 - Predict the Output

items = [10, 0, 5, 0, 7]

found = False
result = None
for item in items:
    if item == 0:
        found = True
        result = item
        break

if found:
    print(f"Found zero at value: {result}")
else:
    print("No zero found")

# Now using idiomatic Python:
zero = next((x for x in items if x == 0), None)
print(f"next() result: {zero}")

print(f"any() result: {any(x == 0 for x in items)}")

Show Answer

Output:

Found zero at value: 0
next() result: 0
any() result: True

The flag variable version correctly finds 0 at index 1 and breaks. result = 0 is set, found = True, and the if found: block prints the result. The next() version returns the first element satisfying x == 0, which is 0. The any() version returns True because at least one element is 0.

The key insight: all three produce the same information (that a zero exists), but the flag version requires 6 lines and two variables, while any() requires one line. If you also need the value, next() replaces the flag + loop + result variable combination.

Level 2 - Debug the Code

Find all anti-patterns in this function and describe each one. You do not need to write the fix.

def send_notification(user, message, is_test=False, verbose=False):
    if user is not None:
        if user.is_active == True:
            if len(user.notification_channels) > 0:
                sent = False
                for channel in user.notification_channels:
                    if channel.type == 1 or channel.type == 2:
                        if not is_test:
                            deliver(channel, message)
                            if verbose:
                                print(f"Sent via {channel}")
                            sent = True
                            break
                        else:
                            if verbose:
                                print(f"Test mode: would send via {channel}")
                            sent = True
                            break
                if not sent:
                    if verbose:
                        print("No valid channel found")

Show Answer

Anti-patterns identified:

1. Pyramid of Doom - Four levels of nesting (if user, if user.is_active, if len(...), for channel, if channel.type, if not is_test). Should be flattened with guard clauses at the top.

2. Redundant condition - if user.is_active == True: should be if user.is_active:. The == True comparison is redundant when the value is already boolean.

3. Redundant condition - if len(user.notification_channels) > 0: should be if user.notification_channels:. Testing non-emptiness does not require explicit length comparison.

4. Magic number condition - if channel.type == 1 or channel.type == 2: uses bare integer literals. Should use named constants or an Enum: if channel.type in (ChannelType.EMAIL, ChannelType.SMS):.

5. Flag variable - sent = False ... sent = True; break ... if not sent: is the classic flag pattern. The else clause of the for loop handles "loop completed without break" natively.

6. Boolean parameter flags - is_test and verbose both create branching throughout the function, making it two (or four) functions in one. Should be split into send_notification() and preview_notification(), with a logger object replacing verbose.

7. Condition duplication - the if verbose: print(...) and sent = True; break logic is duplicated in the is_test and non-is_test branches. The code in both branches is nearly identical - the is_test guard should be extracted before the channel loop.

8. else after return - the nested if/else on is_test inside the channel loop should be guard-clause structured: deliver only if not is_test, then log regardless.

Level 3 - Design Challenge

Refactor the following 55-line function. It contains at least six distinct anti-patterns. Your refactored version should have no nesting deeper than two levels, no flag variables, no magic numbers, no boolean parameter flags, no redundant conditions, and no else after return. Preserve all the business logic.

def process_payment(order, user, payment_method, debug=False):
    if order is not None:
        if user is not None:
            if user.status == 1:
                if not user.is_banned == True:
                    if payment_method is not None:
                        if payment_method.type == 3 or payment_method.type == 4:
                            if len(order.items) > 0:
                                total = 0
                                for item in order.items:
                                    total = total + item.price
                                if total > 0:
                                    charged = False
                                    for attempt in range(3):
                                        try:
                                            result = charge(payment_method, total)
                                            if result.success == True:
                                                charged = True
                                                break
                                        except Exception:
                                            pass
                                    if charged:
                                        order.status = 5
                                        save(order)
                                        if debug:
                                            print(f"Order {order.id} completed")
                                        return "success"
                                    else:
                                        if debug:
                                            print(f"Charge failed for order {order.id}")
                                        return "charge_failed"
                                else:
                                    return "zero_total"
                            else:
                                return "empty_order"
                        else:
                            return "unsupported_payment"
                    else:
                        return "no_payment_method"
                else:
                    return "banned_user"
            else:
                return "inactive_user"
        else:
            return "no_user"
    else:
        return "no_order"

Show Reference Solution

Anti-patterns in the original:

1. Pyramid of Doom (9 levels of nesting)
2. user.status == 1 - magic number (should be UserStatus.ACTIVE)
3. not user.is_banned == True - redundant: not user.is_banned suffices
4. payment_method.type == 3 or payment_method.type == 4 - magic numbers
5. len(order.items) > 0 - should be order.items
6. result.success == True - redundant: result.success suffices
7. charged = False ... charged = True; break ... if charged: - flag variable
8. debug boolean parameter that creates branching throughout

Refactored version:

from enum import IntEnum
import logging

logger = logging.getLogger(__name__)


class UserStatus(IntEnum):
    INACTIVE = 0
    ACTIVE = 1
    SUSPENDED = 2


class PaymentMethodType(IntEnum):
    CREDIT_CARD = 3
    DEBIT_CARD = 4


class OrderStatus(IntEnum):
    PENDING = 1
    PROCESSING = 2
    COMPLETED = 5
    FAILED = 6


SUPPORTED_PAYMENT_TYPES = {PaymentMethodType.CREDIT_CARD, PaymentMethodType.DEBIT_CARD}
MAX_CHARGE_ATTEMPTS = 3


def calculate_order_total(order) -> float:
    """Sum item prices for the order. Pure function - no side effects."""
    return sum(item.price for item in order.items)


def attempt_charge(payment_method, total: float) -> bool:
    """
    Attempt to charge payment_method for total.
    Retries up to MAX_CHARGE_ATTEMPTS times.
    Returns True on success, False if all attempts fail.
    """
    for attempt in range(1, MAX_CHARGE_ATTEMPTS + 1):
        try:
            result = charge(payment_method, total)
            if result.success:
                return True
            logger.warning(f"Charge attempt {attempt} returned non-success: {result}")
        except Exception as exc:
            logger.warning(f"Charge attempt {attempt} raised exception: {exc}")
    return False


def process_payment(order, user, payment_method) -> str:
    """
    Process a payment for an order.
    Returns a status string: "success", or a specific failure reason.
    Raises nothing - all failure modes are communicated via return value.
    """
    # Guard clauses - validate all required inputs
    if order is None:
        return "no_order"
    if user is None:
        return "no_user"
    if user.status != UserStatus.ACTIVE:
        return "inactive_user"
    if user.is_banned:
        return "banned_user"
    if payment_method is None:
        return "no_payment_method"
    if payment_method.type not in SUPPORTED_PAYMENT_TYPES:
        return "unsupported_payment"
    if not order.items:
        return "empty_order"

    total = calculate_order_total(order)
    if total <= 0:
        return "zero_total"

    # Attempt charge - retry logic is encapsulated in attempt_charge()
    if not attempt_charge(payment_method, total):
        logger.error(f"Charge failed for order {order.id} after {MAX_CHARGE_ATTEMPTS} attempts")
        return "charge_failed"

    # Happy path: mark order complete and persist
    order.status = OrderStatus.COMPLETED
    save(order)
    logger.info(f"Order {order.id} completed successfully - total: {total}")
    return "success"

What changed and why:

Change	Anti-pattern removed
Guard clauses at top	Pyramid of doom (9 → 1 nesting level)
UserStatus.ACTIVE enum	Magic number status == 1
if user.is_banned:	Redundant not user.is_banned == True
SUPPORTED_PAYMENT_TYPES set	Magic numbers type == 3 or type == 4
if not order.items:	len(order.items) > 0
if result.success:	result.success == True
attempt_charge() returns bool	Flag variable charged
logger.info/warning	debug boolean parameter flag
calculate_order_total()	Inline loop obscuring intent

The refactored function is 16 lines long (down from 55), has maximum nesting depth of 1, and each guard clause directly maps to one business rule. The charge retry logic is independently testable as attempt_charge(). Adding a new validation rule requires inserting two lines.

Key Takeaways

• The pyramid of doom doubles cognitive complexity with each nesting level - guard clauses invert each condition for early return and flatten the function to depth 1
• Flag variables (found = False) are replaced by for/else, any(), next(), or by extracting to a function and returning from inside the loop
• Using exceptions for normal flow control violates the semantic contract of exceptions - use .get(), in, and hasattr() for expected data lookups
• Condition duplication creates drift bugs when requirements change - extract the predicate to a named function as the single source of truth
• Magic numbers in conditions must be replaced with named constants or enums so the intent is clear and the value is changed in one place
• Boolean parameter flags that control fundamentally different code paths signal that a function should be two functions with descriptive names
• else after return or raise is visual noise - the else is never a guard since the if block already exits; remove it and unindent
• Redundant conditions (if x == True:, if len(x) > 0:) violate Python idiom and should be replaced with direct truthiness tests
• Shadowed loop variables in nested loops cause silent incorrect index usage - always use distinct descriptive names for inner and outer loop variables
• while True without a clear exit is a latent hang - every infinite loop must have a documented termination strategy with a maximum bound or explicit condition
• Measurable metrics that capture anti-pattern cost: cyclomatic complexity (number of paths) and cognitive complexity (path count weighted by nesting depth)