Skip to main content

Refactor Diff Battle

Three real refactor PRs. For each: read the original snippet and the candidate fix. Did the author tackle the worst thing first, or a tertiary smell? Accept or reject; the senior's verdict reveals.

Sibling tools: paste your own code at ✨ Refactor Coach, guess the senior's top finding at 🎯 Refactor Challenge, or replay a prod incident at 🔥 Postmortem Replay.

Battle 1 of 5See the full challenge →

O(n²) lookup loop

Original

# Find each user by id from a big list. Works but slow.

DB = [{"id": i, "name": f"user_{i}"} for i in range(10_000)]

def fetch_users(ids):
    users = []
    for id in ids:
        for user in DB:
            if user["id"] == id:
                users.append(user)
                break
    return users

Candidate refactor

# Candidate refactor v1 — renamed the loop variable.

DB = [{"id": i, "name": f"user_{i}"} for i in range(10_000)]

def fetch_users(ids):
    users = []
    for uid in ids:                # was 'id' — no longer shadows the built-in
        for user in DB:
            if user["id"] == uid:
                users.append(user)
                break
    return users
Battle 2 of 5See the full challenge →

Bare except + mutable default

Original

# Defensive code that swallows the wrong things.

def parse_lines(raw, results=[]):
    for line in raw.split("\n"):
        try:
            results.append(int(line))
        except:
            pass
    return results

Candidate refactor

# Candidate refactor v1 — replaced the mutable default.

def parse_lines(raw, results=None):
    if results is None:
        results = []
    for line in raw.split("\n"):
        try:
            results.append(int(line))
        except:
            pass
    return results
Battle 3 of 5See the full challenge →

Class with no encapsulation

Original

# A "model" with everything public and no validation.

class Order:
    def __init__(self, items, total):
        self.items = items
        self.total = total

    def add_item(self, item, price):
        self.items.append(item)
        self.total = self.total + price

    def apply_discount(self, percent):
        self.total = self.total - self.total * percent / 100

Candidate refactor

# Candidate refactor v1 — privacy via underscores.

class Order:
    def __init__(self, items, total):
        self._items = items
        self._total = total

    def add_item(self, item, price):
        self._items.append(item)
        self._total = self._total + price

    def apply_discount(self, percent):
        self._total = self._total - self._total * percent / 100
Battle 4 of 5See the full challenge →

Timezone-naive datetime

Original

# Stores user signup timestamps. Works on the dev machine.
from datetime import datetime

class UserStore:
    def __init__(self):
        self.users = {}

    def create(self, user_id, email):
        self.users[user_id] = {
            "email": email,
            "signed_up": datetime.now(),
        }

    def signed_up_today(self, user_id):
        u = self.users.get(user_id)
        if not u:
            return False
        return u["signed_up"].date() == datetime.now().date()

Candidate refactor

from datetime import datetime
from dataclasses import dataclass

@dataclass
class User:
    email: str
    signed_up: datetime

class UserStore:
    def __init__(self):
        self.users: dict[int, User] = {}

    def create(self, user_id: int, email: str) -> None:
        self.users[user_id] = User(email=email, signed_up=datetime.now())

    def signed_up_today(self, user_id: int) -> bool:
        u = self.users.get(user_id)
        if u is None:
            return False
        return u.signed_up.date() == datetime.now().date()
Battle 5 of 5See the full challenge →

O(n²) string concatenation

Original

# Render a list of rows as CSV. Slow at 100k rows.

def render_csv(rows):
    out = ""
    for row in rows:
        out += ",".join(str(c) for c in row) + "\n"
    return out

Candidate refactor

def render_csv(rows):
    # Hoisted the comma-join out of the loop body for clarity.
    out = ""
    sep = ","
    nl = "\n"
    for row in rows:
        out += sep.join(str(c) for c in row) + nl
    return out

More battles added as the curriculum grows. Got a PR you want turned into a battle? Email us.