DEV Community

Anas Bahraoui
Anas Bahraoui

Posted on AI-assisted

I Put 3 Bugs in This Pull Request. How Many Can You Catch?

Most coding interview practice asks you to write code.

But here's a different test:

Can you spot what's wrong with code someone else wrote?

Imagine this PR just landed on your desk.

The developer added an endpoint for transferring money between two accounts.

Your job isn't to rewrite it. Review it.

There are 3 important problems hiding in this code:

@app.post("/transfer")
def transfer_money(
    sender_id: int,
    receiver_id: int,
    amount: float,
    db: Session = Depends(get_db)
):
    sender = db.query(Account).filter(
        Account.id == sender_id
    ).first()

    receiver = db.query(Account).filter(
        Account.id == receiver_id
    ).first()

    if not sender or not receiver:
        raise HTTPException(
            status_code=404,
            detail="Account not found"
        )

    if sender.balance < amount:
        raise HTTPException(
            status_code=400,
            detail="Insufficient funds"
        )

    sender.balance -= amount
    receiver.balance += amount

    db.commit()

    return {
        "success": True,
        "sender_balance": sender.balance
    }
Enter fullscreen mode Exit fullscreen mode

Stop here for a second.

If this were a real PR, what would you flag before approving it?

Don't scroll until you've picked your three.


Bug #1: The amount isn't validated

Nothing prevents:

amount = -500
Enter fullscreen mode Exit fullscreen mode

Now look at:

sender.balance -= amount
receiver.balance += amount
Enter fullscreen mode Exit fullscreen mode

A negative transfer increases the sender's balance and decreases the receiver's.

At minimum, the endpoint needs to reject zero and negative amounts.


Bug #2: float is being used for money

amount: float
Enter fullscreen mode Exit fullscreen mode

Floating-point arithmetic isn't appropriate for exact monetary values.

Values such as 0.1 cannot always be represented exactly in binary floating point, which can create rounding errors across calculations.

Money should generally use a fixed-precision representation such as Decimal or integer minor units.


Bug #3: The balance check and update aren't concurrency-safe

This one is easier to miss.

Imagine the sender has $100.

Two $80 transfers arrive at nearly the same time.

Request A reads:

balance = 100
Enter fullscreen mode Exit fullscreen mode

Request B also reads:

balance = 100
Enter fullscreen mode Exit fullscreen mode

Both pass:

if sender.balance < amount:
Enter fullscreen mode Exit fullscreen mode

Both can then proceed based on stale state.

A production implementation needs transaction-level protection appropriate to the database, such as row locking or an atomic conditional update.


Writing code and reviewing code are different skills

That's the part I find interesting.

You can understand Python perfectly and still miss a concurrency bug while reading somebody else's PR.

And in actual engineering work, you're constantly reading code you didn't write.

I wanted a way to practice that deliberately, so I built ReviewIQ.

You choose your:

  • role
  • programming language
  • seniority

It gives you a code review containing 3 planted bugs.

You review the code like a PR, submit your findings, and get scored on what you caught and what you missed.

The first 5 reviews are free and don't require a card:

https://reviewiq-ruby.vercel.app/

Think you'd catch all 3 in the next one?

Try a review and see what you miss.

Top comments (0)