Jagopy/Guides
code Review/edge cases/Null, Empty, and Boundary Edge Cases: The Reviewer's Systematic Checklist
edge cases

Null, Empty, and Boundary Edge Cases: The Reviewer's Systematic Checklist

Empty collections, zero, duplicates, timezones, and unicode boundaries — a fixed checklist for finding the valid inputs a pull request author never pictured.

Reviewer Detection Checklist

0 of 12 verified (0%)

Defect Patterns & Fixes

#1

Aggregation over an empty collection

python
Scenario: A dashboard endpoint computes a user's average review score for the profile header.
def average_score(submissions: list[Submission]) -> float:
    total = sum(s.overall_score for s in submissions)
    return round(total / len(submissions), 1)


# First-ever page load for a new user:
average_score([])   # ZeroDivisionError -> 500 on the profile page
Why this is defective: The empty case is not a rare edge — it is the state of every user on their first visit, which makes it the worst possible place for a 500. The important part of the fix is choosing a meaningful representation: None (rendered as a dash) says 'no data yet', whereas returning 0.0 would say 'this user scores zero' and would drag any downstream average or ranking down with it. Picking the wrong empty value turns a crash into a wrong number, which is harder to notice.
How to Spot It in Reviews:
  • •Any division where the divisor is a len(), count, or size of caller-supplied data.
  • •max(), min(), first(), and [0] on a collection whose emptiness is not established above.
  • •Ask specifically about the newest user, the newest tenant, and the freshly created record.
  • •Check what the chosen empty value means downstream — 0 in an average and 0 in a sum are not the same claim.
#2

Non-idempotent webhook handler

java
Scenario: A payment provider posts a payment.succeeded webhook; the handler credits the customer's wallet.
@PostMapping("/webhooks/payments")
public ResponseEntity<Void> onPayment(@RequestBody PaymentEvent event) {
    Wallet wallet = wallets.findByCustomer(event.getCustomerId());
    wallet.credit(event.getAmount());
    wallets.save(wallet);
    return ResponseEntity.ok().build();
}
Why this is defective: Every mainstream webhook and queue delivers at least once, so a duplicate is a normal input, not an anomaly. Any network blip, timeout, or 500 from this endpoint produces a redelivery, and a redelivery credits the wallet twice. The dedupe must be enforced by a unique constraint rather than a SELECT followed by an INSERT, because two duplicate deliveries can land on two instances simultaneously — otherwise the idempotency check has the same check-then-act race as the bug it is fixing.
How to Spot It in Reviews:
  • •Any handler for a webhook, queue message, or retryable RPC that performs a write with a side effect.
  • •Ask: 'if this exact request arrives twice, what is different the second time?' A correct answer is 'nothing'.
  • •A dedupe implemented as if (exists) return; followed by an insert is still racy — look for the unique constraint.
  • •Check whether the dedupe store has a retention policy; unbounded is a slower defect but still one.
#3

Server clock read inside domain logic

csharp
Scenario: A subscription service decides whether a trial has expired.
public bool IsTrialExpired(Subscription subscription)
{
    return DateTime.Now > subscription.TrialEndsAt;
}
Why this is defective: Three problems ride on one call. DateTime.Now returns the server's local time, so the same subscription expires at different moments depending on which host answers — and shifts by an hour twice a year. Comparing a local DateTime against a stored value of unknown kind is an unchecked assumption the compiler will not flag. And because the clock is read inside the rule, the rule cannot be tested at a boundary without changing the machine's clock, so the boundary never gets a test. Passing time in makes the dependency explicit, testable, and unambiguous about UTC.
How to Spot It in Reviews:
  • •Grep for DateTime.Now, LocalDate.now(), datetime.now(), and time.Now() inside anything that is not a composition root or logging.
  • •DateTime where DateTimeOffset is meant — a timestamp with no offset is missing information.
  • •Ask how a test would assert the behaviour one second before and one second after the boundary.
  • •Date arithmetic that assumes 24-hour days or fixed month lengths breaks on DST and on 31 January.
#4

Truncation that splits a multi-byte character

go
Scenario: A display name is shortened before being written to a fixed-width column.
func truncate(name string, max int) string {
    if len(name) <= max {
        return name
    }
    return name[:max]      // len() and slicing are in bytes
}

truncate("Zoë Ramírez", 4)   // "Zo\xc3" — invalid UTF-8
Why this is defective: In Go, len on a string counts bytes and slicing cuts at byte offsets, so truncating in the middle of a multi-byte rune produces a string that is no longer valid UTF-8. The immediate result is mojibake in the UI; the real damage is downstream, where a JSON encoder, a database with a strict charset, or a search indexer rejects or mangles the record. Naming the parameter maxRunes rather than max is part of the fix — the original defect is really a missing unit on a number.
How to Spot It in Reviews:
  • •Any slicing or substring against a length limit, especially one that matches a database column width.
  • •A limit expressed as a bare number with no unit in its name: bytes, runes, characters, or grapheme clusters?
  • •Test data that is entirely ASCII is a signal the author did not consider it — ask for one accented or emoji case.
  • •Combining characters and emoji mean even rune counts can split a user-perceived character; if display width matters, say so explicitly.

Why edge cases are the defect class reviewers are best placed to find

An edge case is a valid input the author never pictured. Empty list. One element. Zero. Negative. A tenant with no rows yet. A retry that arrives after the record was deleted. A user in a timezone where the day has 23 hours. The code is not wrong in an interesting way — it simply never considered the input, and the author cannot see the gap because they are still holding the picture that produced the code.

The reviewer is not holding that picture. That asymmetry is the whole value of a second reader, and it is why "what happens when this list is empty?" catches more real bugs than any static analysis tool.

What it costs in production

  • Day-one failures. New tenants, new accounts, and fresh environments hit the empty-collection path first. Division by len(items) means the shiniest customer sees a 500 on their first login.
  • End-of-month failures. Date logic that adds a month to 31 January, or assumes every day has 24 hours, breaks on a schedule you can predict and still nobody does.
  • Retry-triggered failures. At-least-once delivery guarantees mean the second copy of a message is the normal case, not the exception. Code that assumes the row still exists, or that the operation has not already run, breaks on the retry.
  • Boundary corruption. Truncating a string at a fixed byte length splits a multi-byte character and produces data the next consumer cannot parse.
  • Scale-triggered failures. A limit that was generous at 100 rows (IN clause size, request body size, page size) becomes a hard error at 10,000.

Edge-case failures cluster at the least convenient moments — onboarding, month end, incident recovery, and traffic spikes — because those are exactly the conditions that produce unusual inputs.

How to spot it in review

Work from a fixed list. Human attention drifts; a checklist does not.

For every collection: empty, exactly one, exactly the page/batch size, and larger than any limit downstream. Look specifically for aggregation — sum / len(items), max(...), items[0], list.First() — all of which have an empty-input failure mode.

For every number: zero, negative, the maximum, and the value that makes a divisor zero. Then ask whether the type can even hold the result — an int counter of bytes overflows quietly in C# and Java, panics in debug Rust, and wraps in release Rust.

For every string: empty, whitespace only, very long, and non-ASCII. Byte length is not character length is not display width. Truncation and fixed-size database columns are where this bites.

For every time value: what timezone is it in, what happens across a DST boundary, and is it derived from the server clock inside domain logic? DateTime.Now, LocalDate.now(), and time.Now() buried in a business rule make the behaviour untestable and environment-dependent.

For every external input: absent field, null, wrong type, duplicate delivery, and out-of-order delivery. If the PR consumes a queue or a webhook, idempotency is an edge case and not an optimisation.

For every state transition: what if it is already in the target state? Cancelling a cancelled order, refunding a refunded payment, and closing a closed connection all need a defined answer.

Note

A quick test for whether the author considered the edge: does the diff contain a test with an empty input, a zero, or a duplicate? Tests are where "I thought about it" leaves a trace.

Fixing it without breaking something else

Handling an edge case means choosing a behaviour, and that choice is a contract change.

  • Returning early on empty changes the caller's contract. Is an empty input a success returning zero, or an error? A payment total of 0.0 for an empty basket may be correct — or it may mean a caller silently charges nothing.
  • Adding validation converts wrong results into errors. That is usually right, but it moves the failure to a new place. Existing callers that were relying on the lenient behaviour will now throw, so check every call site and every retry policy that will now retry a permanent failure forever.
  • Idempotency needs a key and a store, and both need lifetimes. A dedupe table without a retention policy becomes an unbounded table. A dedupe window shorter than the retry window does not dedupe.
  • Clamping hides the caller's bug. max(0, stock) makes negative inventory invisible while leaving whatever produced the negative number in place.
  • Widening a type or a limit has downstream reach. Changing an int column to bigint, or a 255-char field to text, touches every consumer, every serialiser, and possibly an index.

How not to over-engineer the fix

Edge cases invite defensive sprawl. A reviewer who asks for every input to be validated at every layer produces code where the real logic is 10% of the lines and every function re-checks what its caller already checked.

Principles that keep it proportionate:

  1. Validate once, at the boundary. Parse untrusted input into a type that cannot be invalid, then trust that type inwards. A PositiveQuantity beats a quantity > 0 check in nine functions.
  2. Prefer making the state unrepresentable over checking for it. A non-empty list type, a required constructor argument, or a database NOT NULL removes the branch instead of guarding it.
  3. One guard clause, not a nest. If the fix adds three levels of if, the edge case is telling you the function has too many responsibilities.
  4. Do not add a retry/circuit-breaker/queue for an input problem. Infrastructure does not fix an unhandled empty list.
  5. Do not invent behaviour for edges the product has not defined. If nobody knows what a refund on a cancelled order should do, the review comment is "this case is undefined — confirm with product", not a guess implemented in code.

Tip

The cheapest possible edge-case fix is a parameterised test that includes the edge value. If the behaviour is already correct, you have documented it for free; if it is not, you have found the defect without writing any production code.

Comment templates that an agent can act on

  • "sum(scores) / len(scores) divides by zero when a user has no submissions yet — return None for the empty case and let the caller render a dash."
  • "This handler is not idempotent: a duplicate webhook delivery charges twice. Key on event_id and short-circuit on a repeat."
  • "getItems() may be empty here; items.get(0) throws. Guard the empty case before indexing."
  • "LocalDate.now() inside the pricing rule makes this untestable and server-timezone dependent — take a Clock parameter."
  • "Truncating to 255 bytes can split a UTF-8 character; truncate by code points or store the full value."
Previous Guide
Logic and State Mutation Bugs: Finding Correctness Defects in a Pull Request
Next Guide
Swallowed Exceptions and Silent Failures: Reviewing Error Handling That Hides Defects
Interactive Practice

Review Defective Code in the Workbench

Test your ability to spot this defect in our interactive Monaco-powered PR code editor.

Open Workbench