Repair, do not rewrite

Why the rewrite is tempting and usually wrong

You have found the bug. The function is not how you would have written it. Rewriting it will take four minutes and the result will be code you understand completely.

Here is what that trade actually costs. The function in front of you already handles cases you have not thought about: the empty input somebody hit in March, the negative quantity that a refund flow produces, the currency rounding rule that took two days to get right. None of that is documented and some of it is invisible in the code, sitting in an if that looks redundant until you delete it.

A rewrite throws all of it away and replaces it with your model of the problem, which is one afternoon old. The bug you set out to fix does get fixed. What you cannot see is the three you reintroduced.

The default is the smallest change that makes the failing case pass without changing any passing case.

The repair loop

Four steps, in this order, every time.

1. Pin the failure. Write the assertion that fails now. If you cannot, you do not yet know what the bug is.
2. Find the smallest change that makes it pass. Usually one line, often one operator or one guard.
3. Run the other cases. The repair is not done because the new test is green. It is done when the new test is green and nothing else turned red.
4. Keep the assertion. It goes into the suite named after the claim it makes.

The second step is where judgment lives. Minimal does not mean cheapest to type. if page == 0: page = 1 makes one failing test pass and leaves page = -3 broken. page = max(1, page) handles the whole class the contract describes, in the same amount of code.

def page_slice(items, page, per_page):
    page = max(1, page)                # the class, not the one failing value
    start = (page - 1) * per_page
    return items[start : start + per_page]

The test tells you the case. The contract tells you the class. Repair to the class.

Repairs that break something else

Every guard you add is a behavior change for some input, and the inputs you were not thinking about are the ones that bite.

# Bug: crashes on an empty list.
def average(values):
    return sum(values) / len(values)

# Repair A
def average(values):
    if not values:
        return 0
    return sum(values) / len(values)

# Repair B
def average(values):
    if not values:
        return None
    return sum(values) / len(values)

Both stop the crash. They are not equivalent, and choosing between them is not a matter of taste. If the caller displays the result, 0 reads as "the average is zero", which is a claim about data that does not exist. If the caller sums averages, None crashes one line later instead. The contract has to decide, and if the contract does not say, that is the review comment: ask, do not guess.

Interview nuance: in a debugging exercise, saying "I could rewrite this but I would rather make the minimal change and keep the behavior it already has" is a signal that you have maintained code someone else wrote. It is also the answer that keeps you inside the time limit, because a rewrite always takes longer than it looks and the interviewer is watching the clock.