Why Your Code Works But Still Gets Rejected in Code Review
Source: Dev.to
Have you ever submitted a pull request where all the tests pass, the feature works perfectly, and yet… your code review comes back with “Request Changes” in bright red?
Yeah, me too. And it’s frustrating because you’re thinking, “But it WORKS!”
Working code is just the minimum bar. It’s not the finish line.
Let’s talk about the hidden rules of code review that nobody tells you about.
1. Cognitive Load
Your code isn’t read once. It’s read hundreds of times over its lifetime. Every time someone debugs an issue, adds a feature, or tries to understand how the system works, they’re reading your code.
Bad example
d = datetime.now()
y = d.year
m = d.month
dy = d.day
This works, and the tests pass, but every reader has to mentally decode what each variable means. That’s cognitive load.
Better example
today = datetime.now()
year = today.year
month = today.month
day = today.day
It’s self‑documenting. The reader can immediately understand the intent and move on to the actual logic.
Good code doesn’t make you feel smart for writing it. Good code makes the next person feel smart for reading it.
2. Premature Abstraction
Oh boy, have I been guilty of this one. You get excited about design patterns, and suddenly you’re building a DataProcessorFactory with a ProcessorRegistry and a Strategy pattern for a feature that literally runs once a day.
There’s a principle called YAGNI – You Aren’t Gonna Need It.
Abstraction is powerful, but it comes with a cost:
- Every abstraction layer is another place where bugs can hide.
- Every interface is another thing future developers need to understand.
Rule of thumb: Don’t abstract until you have at least three concrete examples of the pattern. Two is coincidence. Three is a pattern.
3. Missing the “Why”
Comments should explain WHY, not WHAT. The code already shows what you’re doing.
// convert to milliseconds
return seconds * 1000;
That comment adds zero value—the code already says it’s a conversion.
// API times out after 5 seconds, so we use 4.5 s to leave a buffer
return seconds * 4500;
Now you’re documenting the business logic and explaining why that specific number matters.
Same for retry logic
// BAD: "Retry 3 times" is obvious from the code.
// GOOD: Third‑party API occasionally fails with 503 errors;
// retrying usually succeeds within 3 attempts.
for (let i
Juniors think about what should happen.
Seniors think about what could happen.
7. Magic Numbers
What does 50 mean? What does 300000 mean? What does 0.0 mean?
(Your original content cut off here—make sure to replace magic numbers with named constants or configuration values, and add comments that explain their purpose.)
Want more real‑world production tips?
Get 25 years of production‑grade experience on my YouTube channel: 🛠️👉
What does 0.75 mean?
if (users.length > 50) paginate();
setTimeout(retry, 300000);
if (score > 0.75) approve();
These numbers might make sense to you right now, but they’re meaningless to anyone else — and they’ll be meaningless to you in three months.
Use named constants
const MAX_USERS_PER_PAGE = 50;
const RETRY_DELAY_MS = 300_000; // 5 minutes
const SUCCESS_THRESHOLD = 0.75;
MAX_USERS_PER_PAGE tells a story. When the product manager asks you to change the threshold, you want to find it quickly and change it confidently. Named constants make that possible.
8. Testing Shortcuts
Tests pass, but do they actually test anything?
expect(result).toBeTruthy();
This test will pass if the function returns 1, true, "hello", or literally any non‑falsy value. It tells you nothing about whether the code is actually working correctly.
Good tests verify specific behavior
expect(result.email).toBe("user@example.com");
expect(result.name).toBe("Jane Doe");
expect(result.id).toBeDefined();
They document what the code is supposed to do. They give you confidence that when you refactor, you haven’t broken anything. Your tests are documentation for future developers — make them count.
The Real Goal of Code Review
Code review isn’t really about finding bugs. Modern testing catches most of those.
Code review is about knowledge sharing. It’s about maintaining code quality over time. It’s about catching edge cases and ensuring long‑term maintainability.
When a reviewer asks you to change something, they’re not saying your code doesn’t work. They’re saying:
- “this will be easier to maintain if we do it this way”
- “this matches our patterns better”
- “future you will thank us for this clarity.”
Before you hit submit on your next PR, ask yourself:
- Can someone understand this code in six months when the context has faded?
- Does it follow the team’s conventions and patterns?
- Is it as simple as possible, but no simpler?
- Does it handle errors gracefully?
Working code is the minimum bar, not the finish line. Great code is code that disappears — code that’s so clear and well‑structured that future developers don’t even notice it. They just understand it and move on.
You’re not writing code for computers. Computers don’t care if your variables are named
xoruserAccountBalance. You’re writing code for humans.
Write code for the tired developer at 2 AM trying to fix a production bug. Write code for the new team member trying to understand how the system works. Write code for yourself in six months when you’ve forgotten everything.
What code review feedback frustrates you the most? Drop it in the comments below.
Note: This article was written with AI assistance to help structure and articulate 25+ years of debugging experience. All examples, insights, and methodologies are from real‑world experience.