TL;DR
- A code review interview tests whether you can explain behavior, find meaningful defects and prioritize clear, actionable feedback.
- Start with the requirement and trust boundaries before discussing naming or formatting.
- Use a small counterexample to prove a suspected defect, then test the corrected behavior rather than copying the implementation into assertions.
- The exercise below includes an intentionally flawed JavaScript function, review notes and a locally tested correction. It is original practice, not an employer's question.
What should you do when the interviewer hands you a pull request?
Read the requested behavior first. Identify the caller, the input contract, the source of trusted data and what the user should observe. Then read the change and its tests with those expectations in mind.
For this original exercise, an account page needs the current user's recent orders. The result should contain between 1 and 100 orders, newest first, with a stable ordering for equal timestamps. The response may expose only the order ID, creation time and total amount. The authenticated identity comes from server middleware; request query parameters are untrusted.
The order collection in the example represents already validated database records with unique string IDs, integer timestamps and integer cent amounts. That assumption keeps the exercise focused. A real endpoint also needs authentication middleware, data access and HTTP behavior that this small function does not implement.
Spend five minutes reviewing the proposed change before reading the findings.
The proposed change: find the problems
function recentOrders(orders, query) {
const limit = Number(query.limit || 10);
return orders
.sort((a, b) => b.createdAt - a.createdAt)
.slice(0, limit)
.filter(order => order.userId === query.userId)
.map(order => ({ ...order }));
}The function is short, but line count is not a correctness measure. Imagine Alice has older orders and Bob has the newest order. With a limit of one, the function takes Bob's order first and then filters it out for Alice. Alice receives an empty list even though she has orders.
Now ask what happens if the caller supplies Bob's user ID in the query. The function has no independent authenticated principal to stop that substitution. This is more serious than the incomplete page. It is a broken access control flaw, and the security engineer interview guide includes a similar sample question about a web view that takes a user ID.
Finally, consider who else holds a reference to the orders array and which fields an order record contains. A database model may include internal notes or payment-related identifiers that do not belong in a public response.
Prioritize the findings by impact
| Finding | Why it matters | What would demonstrate it? |
|---|---|---|
| Identity comes from the query | Caller can choose another user's scope | An Alice request supplying Bob's ID |
| All record fields are returned | Internal fields can reach the client | A fixture containing an internal-only field |
| Limiting happens before filtering | Valid orders disappear from a user's page | Bob's order is newer than Alice's and limit is one |
| Sorting mutates the input array | Other code can observe unexpected reordered state | Compare input order before and after calling |
| Limit accepts loosely converted values | Fractions, negative values or other coercions give unclear behavior | Boundary and type tests against the API contract |
| Timestamp ties lack an explicit key | Results can depend on the incoming order | Same timestamps in different fixture permutations |
The identity and response-field issues should be resolved before release. The ordering and limit behavior are also observable correctness problems. A personal preference about variable names can wait.
Google's engineering guidance on what to review covers design, functionality, complexity and useful tests. Apply that lens to the change in front of you. It does not mean every review must produce a fixed number of comments.
What would constructive review comments look like?
Write about the code, the consequence and the required behavior. For example:
“Required: this user ID comes from the request query. Can we use the authenticated principal supplied by the server? Otherwise a caller can ask for another customer's orders. Please add a test where the requested query identity differs from the signed-in user.”
“Required: the limit is applied before the owner filter. With Bob's newest order and Alice's older order, Alice gets an empty page at limit one. Filter within the authorized scope before choosing the most recent records.”
“Required: please project only the three response fields in the endpoint contract. Copying the full record can expose internal fields added to the database model later.”
These are original comments for this exercise. They describe an observable issue and leave room for the author to implement an appropriate fix. Google's guidance on review comments similarly emphasizes explaining the reason and making severity clear.
A corrected version of the small function
The following version accepts the principal separately and defines a strict decimal limit contract. The caller must supply the real server-authenticated principal; passing an arbitrary object from the browser would still be wrong.
function recentOrders(orders, principal, rawLimit = 10) {
if (!principal || typeof principal.userId !== 'string' || !principal.userId) {
throw new TypeError('Authenticated principal required');
}
if (!['string', 'number'].includes(typeof rawLimit) ||
!/^(?:[1-9]|[1-9][0-9]|100)$/.test(String(rawLimit))) {
throw new RangeError('Limit must be an integer from 1 to 100');
}
const limit = Number(rawLimit);
return orders
.filter(order => order.userId === principal.userId)
.sort((a, b) => b.createdAt - a.createdAt ||
(a.id < b.id ? -1 : a.id > b.id ? 1 : 0))
.slice(0, limit)
.map(({ id, createdAt, totalCents }) => ({ id, createdAt, totalCents }));
}Filtering creates a new array before sorting, so the input array is not reordered. Equal timestamps use ascending order ID as the explicit tie-breaker. That choice is an example contract; another application may choose a different stable key.
The code accepts numeric 10 or the string "10", but rejects "01", whitespace-padded strings and fractional values. That is deliberate, not the only possible API policy. State the contract before arguing about which values should be accepted.
The correction was tested as a local JavaScript function. It does not establish that a production database query is scoped correctly or that an HTTP authentication layer cannot be bypassed.
Which tests are most valuable?
Start with Alice and Bob, not a fixture where every order belongs to the same user. Give Bob the newest record and confirm Alice still gets her newest authorized order at limit one. Then check that Bob's own request returns Bob's record.
Add two Alice records with identical timestamps and different IDs. Shuffle the input and confirm the same output order. Freeze the original array and records to expose accidental mutation during the corrected call, and verify that an internal note is absent from the returned objects.
Test limits 1 and 100, an omitted limit, zero, negative values, 101, a fraction, an empty string and an unexpected object. A user with no orders should receive an empty array. A missing authenticated principal should fail before records are returned.
The useful question is whether these tests would catch the original defects. An assertion that only checks “the result is an array” would pass both implementations while protecting almost none of the requirement. The software engineer interview questions by level include a senior code review answer that asks reviewers to examine test changes as carefully as the code.
How would this change for a database-backed endpoint?
Do not fetch every customer's order into application memory and rely on this exercise function as the production data-access design. Scope the database query to the authenticated owner, project only permitted fields, apply a deterministic order and bound the result at the database boundary.
For pagination, define whether the endpoint uses offsets or a cursor. The API design round in the backend engineer interview guide treats pagination as a frequent probe and explains how cursors cope with inserts and deletes. If using a cursor based on creation time, include the tie-breaker so equal timestamps do not make records disappear or repeat across pages. Also decide how concurrent inserts affect the user's view.
Those are follow-up design questions, not reasons to turn a small review into an unrelated rewrite. Separate fixes required for the proposed behavior from larger changes that deserve their own design discussion.
Is this how to prepare for a Google code review interview?
This exercise can help you practise careful review, including principles in Google's public engineering handbook. It is not evidence that Google uses this question, this format or a particular scoring rubric in an interview. Confirm the actual format with the recruiter or interview instructions.
For a timed rehearsal, explain the requirement, identify the highest-impact defect, show the Alice/Bob counterexample and propose a regression test. Then discuss one tradeoff in the corrected approach. That demonstrates reasoning the interviewer can examine instead of a memorized list of code smells.
Use PhantomCodeAI during permitted preparation to practise delivering those comments clearly and responding when new requirements change your initial judgment.