Long Method code smell: how to spot it and fix it with Extract Method

A code smell is a pattern that hints at a design problem. It is not a bug: the code runs. But it makes the code harder to read, test and change. The Long Method is the most common smell, and it is the first of the "Bloaters", things that have grown too big to handle. The code and output below were run with Python 3 before publishing.

How to recognise it

• You have to scroll to read the function.
• You write comments such as # calculate tax to mark sections inside it.
• It does validation, calculation and formatting in one body.
• A test for one rule must set up everything else too.
• Variables are declared far from where they are used.

There is no magic line count. The better test is: can you say what the function does in one short sentence without "and"?

The fix: Extract Method

Take a block with a clear purpose, move it into a function with a name that says the purpose, and call it. Repeat until the main function reads like an outline. Below, receipt_before does everything, and receipt_after is built from five small pieces.

# ---- BEFORE: one long function does everything ----
def receipt_before(items, country):
    if not items:
        raise ValueError("no items")
    for name, price, qty in items:
        if price < 0 or qty < 1:
            raise ValueError(f"bad line: {name}")
    subtotal = 0
    for name, price, qty in items:
        subtotal += price * qty
    rate = 0.15 if country == "BD" else 0.2
    tax = round(subtotal * rate, 2)
    total = round(subtotal + tax, 2)
    lines = []
    for name, price, qty in items:
        lines.append(f"{name:<10} {qty} x {price:>6.2f}")
    lines.append(f"{'subtotal':<10} {subtotal:>10.2f}")
    lines.append(f"{'tax':<10} {tax:>10.2f}")
    lines.append(f"{'total':<10} {total:>10.2f}")
    return "\n".join(lines)

# ---- AFTER: small functions, each with one job and a name ----
def validate(items):
    if not items:
        raise ValueError("no items")
    for name, price, qty in items:
        if price < 0 or qty < 1:
            raise ValueError(f"bad line: {name}")

def subtotal_of(items):
    return sum(price * qty for _, price, qty in items)

def tax_rate(country):
    return 0.15 if country == "BD" else 0.2

def format_lines(items, subtotal, tax, total):
    lines = [f"{name:<10} {qty} x {price:>6.2f}" for name, price, qty in items]
    lines += [f"{'subtotal':<10} {subtotal:>10.2f}", f"{'tax':<10} {tax:>10.2f}", f"{'total':<10} {total:>10.2f}"]
    return "\n".join(lines)

def receipt_after(items, country):
    validate(items)
    subtotal = subtotal_of(items)
    tax = round(subtotal * tax_rate(country), 2)
    return format_lines(items, subtotal, tax, round(subtotal + tax, 2))

The new receipt_after is short and reads top to bottom: validate, add up, work out tax, format. Each helper can now be tested alone, reused, or changed without touching the others.

Prove the behaviour did not change

A refactoring changes the structure, never the result. Keep the old function while you work and compare:

items = [("notebook", 3.5, 4), ("pen", 0.75, 10)]
print(receipt_after(items, "BD"))
assert receipt_before(items, "BD") == receipt_after(items, "BD")
assert receipt_before(items, "DE") == receipt_after(items, "DE")
for bad in ([], [("x", -1, 1)]):
    for f in (receipt_before, receipt_after):
        try: f(bad, "BD"); raise SystemExit("should have failed")
        except ValueError: pass
print("before and after behave the same")

notebook   4 x   3.50
pen        10 x   0.75
subtotal        21.50
tax              3.23
total           24.73
before and after behave the same

The assertions compare the old and new output for two countries and check that both versions reject empty and negative input. Once the tests pass, you can delete the old version.

How to refactor safely

1. Write a test first, or capture the current output as the expected one.
2. Extract one block at a time and run the test after each step.
3. Give the new function a name that describes the purpose, not the steps (tax_rate, not if_country_bd).
4. Pass in what the block needs and return what it produces. Avoid hidden shared state.
5. Stop when the main function reads like a short outline.

When to leave it alone

• A long list of similar steps (a configuration table or a sequence of simple statements) is often clearer in one place than scattered.
• Do not extract a one-line block into a function just to lower a line count. If the name adds nothing, skip it.

Common mistakes

• Extracting without a test, then changing behaviour by accident.
• Poor names (do_part2) that hide the problem instead of solving it.
• Passing eight arguments to the new function. That is the next smell, the Long Parameter List.
• Moving code into another equally long function.

Main idea: if a function needs comments to mark its sections, those sections want to be functions. Extract them one at a time, keep a test running, and name each piece by its purpose.

---

Code smells and refactoring series, Bloaters: 1 of 5.