Code review exercise: why pagination hides the last records
A team directory component uses Math.floor to calculate page count. With 43 members, the last three silently vanish: the Next button disables one page early, and no one notices because staging had exactly 20 people. This exercise trains you to catch off-by-one pagination bugs by testing the code against non-round numbers before you approve. It is useful for anyone who reviews frontend code, especially pull requests where the author says "verified by paging through the list" and the list happened to divide evenly.
I once spent an embarrassing amount of time debugging a support ticket that said "new hires don't show up in the company directory." The directory was there. The people were in the database. Search found them. But if you just paged through the list, the last handful of employees were invisible. The pagination controls said you'd reached the end, and the pagination controls were lying. The root cause was one wrong rounding function on one line, and it had been in production for months because every test environment had a nice round number of seed users.
This is not a rare species of bug. Atlassian's Jira REST API has had recurring reports of pagination returning incomplete results, and anyone who has built a paginated list has probably shipped a version of this at least once. The frustrating part is that it works perfectly during development and demo, because development data is tidy. The bug only bites when production gets messy, which is to say, immediately.
The requirements
The task is to add pagination to a team directory component. The spec is short:
- Show the team directory 10 members per page.
- Prev/Next controls, disabled at the first and last page.
- Show a "Page X of Y" indicator.
- Every member must be reachable through the pagination.
Context from the PR description: the members list comes from an org API and grows as people join. Teams range from a handful to a few hundred people. The author says they verified by paging back and forth through the 20-member staging directory.
The PR
One new file, straightforward React component:
src/components/TeamDirectory.jsx
+28
Clean looking code. Good use of slice for windowing, accessible nav landmark, disabled states on both buttons. The kind of PR that gets a quick approve if you're scanning between meetings. But read line 7 one more time.
Try it yourself first
The same PR is a live exercise on DiffDojo.
Leave your own inline comments and verdict before reading the walkthrough below. Free, no signup.
Review this PR →↓ Walkthrough and solution below ↓
Twenty members, zero problems
The PR description says "verified by paging back and forth through the 20-member staging directory." Let's take the author at their word and check the math. With 20 members, Math.floor(20 / 10) gives pageCount = 2. The Next button disables when page >= pageCount - 1, which is page >= 1. So you can visit page 0 (members 1 through 10) and page 1 (members 11 through 20), and then Next greys out. That's correct. Every member is reachable. The indicator reads "Page 1 of 2" and "Page 2 of 2." All good.
Now imagine the team hires three more people and the directory grows to 43 members. Math.floor(43 / 10) is 4. The Next button disables at page >= 3, which means the last page you can visit is page 3 (0-indexed), showing members 31 through 40. Members 41, 42, and 43 exist in the data. The slice would happily show them if you could get to page 4. But the navigation won't let you, because pageCount says there are only 4 pages, and 4 pages of 10 is 40 members, and the last three got rounded away.
The part where it gets worse
The 43-member case is bad enough, but the small-list behavior is genuinely strange. With 7 members, Math.floor(7 / 10) is 0. The indicator reads "Page 1 of 0." The Next button's condition is page >= -1, which is always true, so Next is permanently disabled. That's accidentally correct for a single page of results, but "Page 1 of 0" is nonsense that would confuse anyone who reads it. And the Prev button is disabled because page === 0, which is fine, so the user is stuck on the one page that does show their 7 members. The data is visible, but the UI is telling you it shouldn't exist.
With 0 members, pageCount is also 0, the list is empty, and the indicator says "Page 1 of 0." No crash, but also no indication that the list is empty on purpose rather than broken. This is the kind of thing that generates support tickets.
The comment
Math.floor drops the partial last page. With 43 members this gives pageCount = 4, and the Next button disables at page 4, so members 41 through 43 are unreachable. This violates the "every member must be reachable" requirement. Needs Math.ceil here, and probably Math.max(1, ...) to keep the indicator sane when the list is empty or smaller than one page.
The verdict is request changes. There's only one finding here, but it directly violates a stated requirement, and the data loss is silent: no error, no empty state, just people who happen to be at the tail of the list becoming invisible. The people who vanish are always the ones at the end of the array, which in a directory sorted alphabetically means everyone whose last name starts with a letter near the end of the alphabet. In a directory sorted by join date, it's the newest hires, the people most likely to be looked up. The rest of the component is fine. The slicing logic, the disabled states on Prev, the key prop, the aria label on the nav: all correct. It's just the one line of arithmetic that's wrong.
The fix
Replace line 7:
Math.ceil rounds up, so 43 members gives pageCount = 5, and the fifth page shows members 41 through 43. The Math.max(1, ...) wrapper handles the empty-list edge case: with 0 members, Math.ceil(0 / 10) is 0, and showing "Page 1 of 0" is confusing. Clamping to 1 means an empty directory says "Page 1 of 1" with an empty list, which is at least not self-contradictory.
The check that catches it
Here's a quick way to verify the fix works and the original doesn't. You can run this in Node or paste it into a browser console:
Output:
0 members: buggy shows 0, fixed shows 0
7 members: buggy shows 0, fixed shows 7
10 members: buggy shows 10, fixed shows 10
20 members: buggy shows 20, fixed shows 20
43 members: buggy shows 40, fixed shows 43
The buggy version loses all 7 members when the list is smaller than a page, and loses 3 out of 43 when the count doesn't divide evenly. The fixed version reaches every member for every input. With exactly 10 or 20, both versions produce the same result, which is precisely why the 20-member staging test passed.
The habit worth keeping
Any time you see integer division controlling a loop bound, a page count, or a slice window, plug in a number that doesn't divide evenly and trace what happens. The author's test data was 20 members. If you mentally substitute 21, the bug is visible in under ten seconds. You don't need to run the code. You don't need to set up a test harness. Just read the arithmetic, pick an ugly number, and do it in your head. The same reflex catches fence-post errors in chunked uploads, batch processing, and anywhere else a remainder gets quietly discarded. Round numbers are the enemy of boundary testing, and real data is almost never round.
Read next LeetCode for code review: where to practice reviewing pull requests