The nightly fraud job that never ends
Priya maintains a nightly Python job for an online store. It walks through that day's orders and flags any order with a total below £25 for fraud review. Today's list holds 8 orders. Order 4 was cancelled, so its total is None. The job uses a while loop with a manually incremented index i, because the previous developer also wanted to remove flagged orders from the same list as it went. The body does three things in order: (1) read orders[i]; (2) if the total is None, continue; (3) if the total is below 25, append the order to a flagged list; then i += 1 at the end of the body. Last night the job ran for 90 minutes without finishing and the on-call engineer killed it. In staging the job hangs whenever a cancelled order is present; with the cancelled order removed it finishes in under a second and flags 3 orders. Priya's manager wants cancellations skipped rather than flagged, and the nightly run to finish in under 2 seconds for up to 50,000 orders.
1. Explain why the job never terminates and give the smallest correct fix, plus one structural change that would make the bug impossible to reintroduce silently.
Read the worked answer
The hang is caused by continue skipping the increment. On the cancelled order, execution jumps back to the while i < len(orders) test with i unchanged, re-reads the same cancelled order, hits continue again, and never reaches i += 1. The loop makes no progress, which is why it only appears when a cancelled order exists and why it cannot be explained by slow work: one order cannot cost 90 minutes.
Smallest fix: advance the index before leaving the iteration.
while i < len(orders):
order = orders[i]
if order['total'] is None:
i += 1
continue
if order['total'] < 25:
flagged.append(order)
i += 1
Structural fix: iterate with for order in orders: so there is no index to forget, and keep continue for the cancelled case. If the index is genuinely needed for removal, make progress explicit, e.g. assert i increases each pass, or add a loop budget that raises instead of hanging. Note that for only helps because the code no longer removes items during iteration; the removal design is the next question.
2. A teammate proposes keeping the current design but calling `orders.pop(i)` when an order is flagged, and not incrementing `i` after a pop. Evaluate this against the manager's 50,000-order performance target.
Read the worked answer
Correctness first: pop(i) shifts every later element one position left, so the element that was at i+1 is now at i. If the code pops and does not increment, the next iteration re-reads that shifted element, so nothing is skipped. That part is coherent. But the same body must still handle two other exits: a skipped cancellation (increment, no pop) and a clean unflagged order (increment, no pop). The index now advances on some paths and not others depending on an unrelated condition, which is exactly the kind of state machine that produced the original hang. A future edit that reorders the branches reintroduces the bug.
Performance: pop(i) is O(n) because of the shift, so removing k items from n is O(k·n). With 50,000 orders and even a few hundred flags this is hundreds of millions of element moves, and it will not reliably meet a 2-second budget. Building a new list is O(n) total.
Recommendation: keep flagged orders in a separate list and, if the source list must shrink, rebuild it once at the end (orders = [o for o in orders if o['id'] not in flagged_ids]) or simply never mutate it. This is both simpler to reason about and linear in the input size.
3. Write the tests that would have caught both the hang and the removal bug. State what makes each test able to fail.
Read the worked answer
A hang is not a failing assertion, so the test harness must turn it into one: use a per-test timeout (e.g. pytest-timeout) and treat expiry as failure. Without that, CI would hang rather than report red.
Cases to cover, each with a hand-checked expected result:
- Cancelled order first, in the middle, and last. Position matters: if the cancelled order is last, the loop exits on the length check and the missing increment is invisible. A middle-position case is the one that actually hangs.
- Cancelled order plus a low-total order after it: assert the flagged list is exactly the expected set. This catches
pop-style removal that silently skips the element that shifted into the vacated index. - No cancelled orders at all: assert 3 flagged, so a fix that drops the cancellation branch entirely is also caught.
- A size/perf guard: 50,000 synthetic orders with a time budget of 2 seconds, run in CI as a slow marker. This catches an O(n²) removal approach that is correct but too slow.
Each test needs a concrete assertion on the returned data (exact IDs, exact count), not just assert result, and should be run in the same session as the others so shared module state would also surface.