The Art of Code Review

Lessons from reviewing production code in fintech, Web3, and enterprise systems, where a missed bug costs more than a bad PR comment.

• 7 min read
DevelopmentBest PracticesTeam CultureCode Quality

The Art of Code Review

Working across fintech, Web3, and enterprise systems has taught me that code review is the last line of defense before code reaches production, not a gatekeeping ritual. In payment systems and blockchain applications, a bug doesn’t just annoy a user. It can move their money somewhere it shouldn’t go.

Why code reviews are non-negotiable

In high-stakes environments, code reviews serve as:

  1. Risk Mitigation: Catch security vulnerabilities, logic errors, and edge cases before they reach production
  2. Knowledge Distribution: Prevent single points of failure by ensuring multiple people understand critical systems
  3. Standards Enforcement: Maintain consistency in architecture, patterns, and quality across a growing codebase
  4. Continuous Learning: Both reviewer and author learn from the exchange

What to review (priority order)

Critical: security and financial logic

In Payment Systems:

  • Validate all monetary calculations (rounding, currency conversion, fee calculations)
  • Check for race conditions in transaction processing
  • Verify authentication/authorization on sensitive endpoints
  • Ensure PCI-DSS compliance for card data handling

In Web3 Applications:

  • Validate wallet connection security
  • Check for reentrancy vulnerabilities in contract interactions
  • Verify transaction signing flows
  • Ensure proper error handling for blockchain failures

Real Example from Production:

// ❌ Dangerous: Floating point arithmetic with money
const total = amount * exchangeRate + fee;

// ✅ Safe: Use integer arithmetic (smallest currency unit)
const amountInPence = Math.round(amount * 100);
const feeInPence = Math.round(fee * 100);
const totalInPence = Math.round(amountInPence * exchangeRate) + feeInPence;

Important: architecture and performance

State Management:

  • Are API calls properly cached to avoid unnecessary network requests?
  • Is loading/error state handled consistently?
  • Are optimistic updates used where appropriate?

Component Structure:

  • Is the component too large? Should it be split?
  • Are concerns properly separated (presentation vs logic)?
  • Is the component reusable or overly coupled?

Real Example from MonieWorld:

// ❌ Poor: Fetching on every render
function RecipientList() {
  const [recipients, setRecipients] = useState([]);

  useEffect(() => {
    fetch('/api/recipients')
      .then((r) => r.json())
      .then(setRecipients);
  }, []); // Still refetches on remount
}

// ✅ Better: Use React Query for caching
function RecipientList() {
  const { data: recipients, isLoading } = useQuery({
    queryKey: ['recipients'],
    queryFn: fetchRecipients,
    staleTime: 5 * 60 * 1000, // Cache for 5 minutes
  });
}

Nice-to-have: style and conventions

Let automation handle these:

  • Code formatting (Prettier)
  • Linting rules (ESLint)
  • Type coverage (TypeScript)

The review process that works

1. Automated checks run first

Never review code manually until CI passes:

  • Unit tests (Jest, React Testing Library)
  • E2E tests (Playwright)
  • Type checking (TypeScript)
  • Linting (ESLint)
  • Coverage thresholds

From MonieWorld CI Pipeline:

  • 85%+ test coverage required
  • Zero TypeScript errors
  • All Playwright E2E tests passing
  • SonarQube quality gate passed

2. Author self-reviews

Before requesting review, I always:

  • Read my own diff top to bottom
  • Remove console.logs and debugging code
  • Verify all test cases pass locally
  • Add screenshots for UI changes
  • Write meaningful PR description and changes

PR Template That Works:

## Problem

Users couldn't see their card details without multiple authentication prompts

## Solution

Implemented 2FA with auto-purging after 60 seconds

## Testing

- Unit tests: Card component, security logic
- E2E tests: Full card viewing flow with 2FA
- Manual testing: Tested on 3 card types

## Screenshots

[Before] [After]

3. Reviewer checklist

When reviewing, I systematically check:

Correctness:

  • Does the code do what the PR says it does?
  • Are edge cases handled? (null, undefined, empty arrays, errors)
  • Are loading and error states managed?

Security:

  • Is user input validated and sanitized?
  • Are authentication checks in place?
  • Is sensitive data (passwords, tokens, card numbers) properly handled?

Testing:

  • Do tests actually test the right things?
  • Are happy paths and error cases both covered?
  • Are tests readable and maintainable?

Maintainability:

  • Will this be understandable in 6 months?
  • Are variable/function names descriptive?
  • Is there unnecessary complexity?

4. Giving effective feedback

Use Categories:

  • 🔴 Blocking: Must be fixed (security, bugs, breaking changes)
  • 🟡 Important: Should be addressed (performance, maintainability)
  • 🟢 Suggestion: Nice to have (optimizations, preferences)

Be Specific:

❌ "This won't scale"
✅ "This fetches all recipients on every search. Consider implementing
pagination or debounced search to reduce API load."

❌ "Bad naming"
✅ "Consider renaming `handleClick` to `handlePaymentSubmit` for clarity
about what action is being performed."

Explain the Why:

// Not just what, but why
"Let's use React Query here instead of useState + useEffect because:

1. Automatic caching reduces API calls
2. Built-in loading/error states
3. Automatic refetching keeps data fresh
4. Better TypeScript support"

Common pitfalls (and how to avoid them)

Perfectionism paralysis

Requesting changes just to make already-good code “perfect” stalls a PR for no real benefit. Before you block it, ask whether the change actually improves correctness, security, or maintainability. If it doesn’t, it’s a suggestion, not a blocker, and the comment should say so.

Rubber stamping

Approving a PR without reading it is worse than not reviewing it at all, because it gives everyone false confidence that a second pair of eyes checked the logic. If you don’t have time for a real review, say that out loud. Delaying a PR by a day costs a lot less than shipping an unreviewed bug into a financial system.

Scope creep in reviews

“While you’re here, can you also refactor this unrelated thing?” is how a five-line fix turns into a three-day PR. File a separate ticket instead, and keep the review focused on what it was actually opened for.

Tools that actually help

From production experience:

  • SonarQube: Catches code smells, security vulnerabilities, and technical debt
  • GitHub Actions / Jenkins: Automated CI/CD prevents broken code from merging
  • Playwright: E2E tests catch integration issues that unit tests miss
  • React Testing Library: Ensures components work from user perspective
  • TypeScript: Catches type errors before runtime

Real-world impact

At MonieWorld, implementing rigorous code review practices contributed to:

  • 99.6% transaction success rate in production
  • 90% reduction in production bugs compared to pre-review baseline
  • Zero financial discrepancies since launch
  • Zero security vulnerabilities in card management systems

In Web3 at Unlockd, code reviews caught:

  • Smart contract interaction bugs that would have lost user funds
  • Wallet connection race conditions causing failed transactions
  • Gas estimation errors leading to stuck transactions

Conclusion

Code review isn’t bureaucracy. It’s the last check before money, sensitive data, or a blockchain transaction goes out broken. The best review catches real issues before production, teaches the author and the reviewer something, holds the line on quality without blocking progress, and keeps excellence the norm instead of the exception.

Every PR you approve is code you’re putting your name on. Review it like that’s true, because it is.