Code review questions.
76 code reviewquestions from the bank — open to read. Pick one and practice it out loud; a coach note comes back in seconds.
Learn the ideas first
All 76 questions
A teammate's pull request adds a function that converts a list of timestamps into a list of dates by truncating the time portion. Walk me through what you'd check before approving.A PR introduces a function that retries failed HTTP requests up to 3 times. Walk me through the edge cases you'd push on.A PR adds 200 lines of valid but dense code into a single function. The reviewer feels something's off but can't articulate it. How would you give the feedback?A PR adds a substantial new feature but its tests only cover the happy path. How do you frame the feedback?A PR renames a long-standing variable from data to records. The rename is technically clearer but touches 300 files. How would you respond?A PR wraps every external API call in a try-catch that logs and returns null on error. Walk me through what's worth raising.You're reviewing a PR from a senior engineer and you've spotted a real bug. How do you frame the comment?A PR adds a function that filters a list using parallel streams. Walk me through what subtle issues you'd look for.A PR sorts a list of users by 'last_active'. The field is sometimes null. What questions do you ask?A PR uses a clever one-liner instead of an existing utility function the team agreed on. The one-liner is shorter. What do you say?A PR adds a small change that requires touching code with no existing tests. Where do you draw the line on test scope?A PR uses floating-point math to compare prices. The author argues the values are always under a million so precision doesn't matter. How do you respond?A PR throws a generic exception in three different failure modes. Each call site has to inspect the message to decide what happened. How would you push back?A junior engineer's PR rewrites a working module they didn't need to touch. The rewrite is fine but unnecessary. What's your comment?A PR caches API responses indefinitely. The author says the responses don't change often. What do you ask?A PR parses user-supplied JSON without specifying limits. What concerns would you raise?A PR introduces a new abstraction that doesn't quite fit any of the existing patterns. The code works. What do you do?A PR's test relies on Thread.sleep to wait for an async result. What concerns would you raise?A PR adds three new boolean parameters to an existing function: 'force', 'silent', and 'strict'. What's your feedback?A PR uses a Set to deduplicate user objects in a list. The Set treats objects as distinct unless they're identical references. What do you raise?You'd block a PR for a real concern, but the author has worked on it for two weeks. How do you write the comment?A PR processes a list of orders one by one in a loop, calling an external API per iteration. What's your concern?A PR introduces a 'fallback to default' on a database read failure. Walk me through whether that's the right call.A PR writes to a shared mutable map in a multi-threaded path. The author says reads are 'mostly fine'. What do you say?A PR adds a new utility but tests only check that it returns the right type. How do you respond?You'd recommend a deeper refactor than this PR is scoped for. The work is real but tangential. How do you make the suggestion without blocking?A PR uses the name 'handle' for a function that loads, transforms, and saves data. What do you suggest?A PR adds a feature that returns a 'recent activity' list. There's no maximum size. What's your concern?A PR's logic is correct but the function does five things. How do you push for separation without dictating the exact decomposition?A PR adds a feature behind a flag. The flag check happens inside three different layers of the code. Walk me through your concerns.A PR adds a function that calculates the average of a list of numbers by summing them and dividing by the length. The function returns 0 when the list is empty. What would you point out?A PR adds a function that takes a user's email and sends a password reset link. The function doesn't validate the email format before sending. What edge cases would you ask about?A PR adds a function with variables named 'd', 'tmp', and 'res'. The logic is correct but hard to follow. How would you phrase feedback to make it constructive?A PR adds a new endpoint that accepts a date range filter. The tests verify it works with valid dates but don't check what happens when start_date is after end_date. What's your feedback?A PR adds a function called 'process' that reads a file, parses JSON, validates fields, and writes to a database. What naming concern would you raise?A PR adds code that reads from a configuration file but doesn't handle the case where the file doesn't exist. Walk me through what you'd check for.A PR adds a search function that loops through all records and filters in memory. It works fine in the test database with 50 records. What would you ask about?A teammate opens a 600-line PR and mentions most of it came from an AI coding assistant. The code looks plausible and the tests pass. How does that change your review, if at all?A PR's tests were auto-generated, and each one asserts whatever the function currently returns — including one case whose expected value looks wrong. What's your review comment?You spot what looks like a live API key committed in a PR's config file. The author says it's only for testing and they'll remove it before merge. Walk me through your response.A PR builds a SQL query by concatenating a user-supplied search string into the WHERE clause. The author points out the input is validated on the frontend. What do you say?A PR adds a new third-party package to save roughly twenty lines of code. How do you decide whether that trade is worth it?A PR ships a database migration that renames a column together with the code change that uses the new name. What's the deployment problem hiding here, and what do you ask for?A PR checks a coupon's remaining-use count with one query, then decrements it with a second write if it's positive. Two reviewers already approved. What do you flag?To debug a flaky integration, a PR adds logging of full request and response bodies on every call. What would you push on before approving?You're assigned a 3,000-line PR that mixes a mechanical refactor with real behavior changes. Line-by-line review isn't realistic. What do you actually do?A PR fixes a real bug but also marks two flaky tests as skipped to get CI green. How do you respond?You're reviewing code that builds an LLM prompt by inserting user-supplied text directly into the system instructions, then acts on the model's reply. What risks do you raise?A PR sets a 30-second timeout on an upstream call that runs inside a request handler with a 10-second SLA. Nobody else flagged it. What's your comment?You're the PR author this time. A reviewer leaves fifteen comments and you disagree with several. How do you decide what to push back on, and how do you do it?A PR hardcodes the number 86400 in four different places. The logic is correct. What do you suggest, and why does it matter beyond style?A PR adds a new function that concatenates user-supplied strings into a SQL query using template literals. The tests pass. What security concerns do you raise?A PR upgrades a transitive dependency from 2.1.4 to 2.1.5 to fix a low-severity CVE. The direct dependency we use is still pinned to 2.1.3. What do you check before approving?A junior engineer's PR has 15 commits with messages like 'fix', 'wip', and 'actually fix it now'. The code itself is good. Do you ask them to squash? How do you explain why?A PR introduces a caching layer that stores user permissions in Redis with a 1-hour TTL. Walk me through the edge cases around stale data and revoked access.A PR adds pagination to an endpoint but the new page_token parameter is optional and defaults to fetching all records. What feedback do you give about API design?A PR contains code that looks suspiciously like it was AI-generated: variable names are generic, there are unused imports, and the structure mimics a textbook example. How do you approach this?A PR refactors a critical payment processing function but doesn't update the existing unit tests—they still pass by mocking the same internal methods. What's your concern?A PR adds input validation that rejects any string containing '<script'. The author says this prevents XSS. What additional concerns or gaps do you raise?A PR changes a function signature from processData(data) to processData(data, options = {}) for backward compatibility. Three calling sites pass undefined as the second argument. What do you say?A PR adds error handling that catches all exceptions, logs them to Sentry, then re-throws. The author argues this gives better observability. What trade-offs do you discuss?A PR introduces a new microservice dependency that duplicates functionality already provided by an existing internal service. The new one has better docs. How do you frame this concern?A staff engineer's PR implements a complex algorithm without comments. When asked, they say the code is self-documenting. You disagree. How do you push back?A PR adds a batch processing job that loads all user records into memory, transforms them, then writes back. It works fine in staging with 10K users. What do you flag?A PR renames isValid to checkValid but the function still returns a boolean. Three teammates have already approved. Do you block on this naming inconsistency?A PR adds a feature flag check inside a tight loop that processes millions of records. The flag is evaluated fresh on every iteration. What performance concern do you raise?A PR updates our Dockerfile to use latest instead of a pinned base image version. The author says this ensures we always get security patches. How do you respond?A PR adds comprehensive integration tests but no unit tests. The author argues integration tests provide better coverage. What's your take on the testing strategy?A PR hardcodes an AWS region as us-east-1 in a new service. The rest of the codebase reads it from environment variables. Do you ask them to change it?A PR adds logging that includes the full request payload, which may contain PII. The logs go to a third-party service. Walk me through your concerns.A PR implements a rate limiter using an in-memory counter. It works in development. What questions do you ask about production deployment with multiple instances?A PR from a contractor includes a new third-party library you've never heard of. It has 47 GitHub stars and was last updated 8 months ago. How do you evaluate whether to approve?A PR adds a TODO comment saying 'refactor this later' in code that's about to be released to production. The code works. Do you require them to address it now or let it go?A PR copies 80 lines of code from another file and modifies 3 lines instead of extracting a shared function. The author says they might diverge later. What do you recommend?A PR deletes error handling around a third-party API call because 'the service has 99.9% uptime so we don't need it'. How do you frame your feedback?A PR includes AI-generated code with a comment block that says 'Generated by GPT-4' and a detailed prompt. The code works and has tests. What's your review stance?