How To Review Code That Deals With Money
You're probably focusing on the wrong things. It doesn't look as good as you think.
You’re a code reviewer, not a code writer.
Even before AI started writing code for us, we didn’t write much. Mileage may vary, but you’re probably spending around 75 percent of your time reading existing code. Writing brand new code? Maybe 5 percent.
We rarely write code from scratch.
And yet, this rare act of typing brand new code dominates our efforts to “get better” as engineers. We build side projects, perform mock interviews, and obsess over blog posts about teams rewriting their code in Rust.
Saving time on rare tasks isn’t as valuable as shaving even a little bit off the ones you do all the time.
What shocks me is how little time engineers spend in getting better at reviewing code.
Worse: now that AI has taken over some part of that tiny fraction of work that we spend writing, we’re still obsessed with it.
And the stakes are even higher when you’re building software that moves money around. There, hallucinated code is unacceptable.
I was working on something but they wouldn’t let me finish it. So, take a look at it.
Be careful.
— Margin Call (2011)
(Margin Call, the only movie in history that revolves around a code review).
I’m Alvaro Duran, and this is The Payments Engineer Playbook. You’re already subscribed to free newsletters that “teach” you how to get a job as a software engineer.
But you don’t want to get a job; you already have one. What you want is to learn how to be great at your job. Especially as a payments engineer, where stakes are sky high, and the margin for errors is razor thin.
In The Payments Engineer Playbook, we investigate the technology that transfers money. All to help you become a smarter, more skillful and more successful payments engineer. And we do that by cutting off one sliver of it and extracting tactics from it.
Today, we’re looking at code reviews. How to do them properly, systematically, and efficiently.
I’m going to share a 4-step process you can use on your next pull request, so you can:
Gain context quickly
Spot errors earlier
Focus on the code that matters most
And, as a nice bonus, become a better engineer faster
After all, reviewing code is 75 percent of your job. Let’s dive in.
1. Read The Test Files First
Most engineers start reviewing a pull request where they shouldn’t.
And it’s not really their fault. It’s just that the files they should be looking at first are at the bottom. I mean the test files.
Tests drive much of the code review process, because if tests are named properly, they give the most context.
So the first thing I do is check the names of the tests. And then I check what they’re all about. That’s it.
Are they meaningful? (as in, “Can I guess the Given/When/Then just by looking at the name?”)
Can I understand, based on the tests, what the goal of this code change is?
Do I know what the code is supposed to solve?
This, by the way, is counter to what you probably do, which is ignoring the tests. By the time you get to the test files, your mental energy is depleted, and you rationalize not even looking at them because that’s code that won’t run in production.
That’s a missed opportunity. Reviewing tests pays off immediately, because it informs the rest of the review.
This first step, reading the test names and their content, gives you enough context to understand what the code changes are meant to address, and what corner cases have been considered.
Which brings me to step 2.
2. Check The Issue Description
Few people read the test files. Almost nobody looks at the issue description when reviewing a pull request.
It’s as if reviewers expect the code to reveal the purpose of the ticket as they read along.
But reading the issue description, especially after reading the test names, answers an important question: is the pull request really addressing the real issue?
The engineer who wrote this code read the issue first, then implemented the changes. So a good reviewer must retrace the steps to make sure that this pull request is solving a problem that actually exists, and not something close enough.
The reason there’s a gap between the requirements and the implemented solution often comes down to the engineer not understanding what the task was really about.
In other words: the issue description is often vague, and the engineer reads it only once before jumping to implementing its solution.
Very few people will actually refer to the requirements document as they work, preferring to work with their memory of what the document says.
— Exploring Requirements, by Donald C Gause, Gerald M Weinberg
Having read the test files and the issue description, ask yourself:
Are the issue description and the tests aligned?
What is ambiguous about the issue, and how it was addressed in the tests?
What do you think wasn’t addressed, or was addressed incorrectly, in the tests?
2.1 (If needed) Check The Provider’s Documentation for Inconsistencies
A significant chunk of any payment system’s codebase relies on external providers.
By that, I don’t mean external libraries, for which a good type system and a modicum of common sense are good remedies against bugs. I mean making requests to external systems, beyond your control, usually in the form of a REST API call.
If the code change that’s in front of you makes such requests, you must check what the provider’s docs have to say about it. This is the primary source of hallucinated code, by a mile.
So, on top of the issue description, make sure you’ve checked what the provider’s docs have to say about this particular code change. How to do it, what the request must look like, and what the response will be.
The test doubles should be absolutely precise in this case. Anything but won’t cut it.
Don’t skip this.
At this point, you may decide that what you have is enough to stop the code review. Even if some people may think you haven’t even started (you just read some test files that nobody looks at, and the JIRA ticket).
But you may decide that the issue was clear enough, and you agree with how it was approached in the test files.
Either way, you’ve gathered enough context on what you need to check about the pull request.
That brings us to step 3.
3. Jot Down How You Would Make The Tests Pass
This is my favorite part of the whole review process.
That’s when I close the page, take some pen and paper, and write a few bullet points on how I would make the tests pass.
It doesn’t have to be proper code. It doesn’t even have to be code. Just a few sentences stating how would you describe to another engineer what would you do to finish off the task.
After all, you have all the context you would’ve had if you implemented the task yourself. Why not spending 1 minute giving it a try?
This is an extremely rewarding exercise. Not only it is something incredibly fun to do (especially if, like me, you don’t get to write code as much as you’d want to), it also stretches your ability as an engineer.
Whoever wrote it took way longer than this to implement it, and you’re getting just as much experience in a couple of minutes.
Jotting down how you would approach the problem will give you
A baseline of what the solution should look like
A dose of interval training for your problem solving skills
Better yet: you may be tempted to paste this into a prompt chat and watch your favorite AI build the solution.
Don’t do that just yet. Read the last step below first.
4. Check the Code Against Your Own Notes
This is where most people start their review process. Notice how much work you’ve done already, and how better prepared you are.
Here’s the trick: don’t read attentively. Just skim.
You’ll notice how your understanding of the code is so much richer and deeper than it would’ve been if you’d just jumped to reading it right away.
You understand the reasoning behind every line, because it aligns or deviates from what you would’ve done. You dismiss what matches your expectations, and narrow down on what surprises you.
It’s like checking an exam’s answers right after you’ve submitted. You appreciate the nuances, and truly engage with what you’re reading.
Moreover, you’ve done the work. You’ve paid respect to your teammate’s work, and it pays off tremendously.
By now, you know what to do:
You can spot errors earlier
Your comments are more nuanced; you may even have suggestions that are immediately applicable
You spend less time on the boilerplate and narrow down on the code that actually matters
You learn from what your teammate has done, because it is often their approach that’s superior to the one you came up with
And then, something very magical happens: you realize that you’ve spent quite some time reviewing this pull request.
And yet, time didn’t feel like passing.
Code Review Driven Development
We do an enormous amount of trading.
There’s billions of dollars of nominal value kind of sloshing back and forth in the systems that we build. What this means is that we are very nervous about technological risk, because there’s no faster way to light yourself on fire than to write a piece of software that makes the same bad decision over and over in a tight loop.
And as you make your systems faster and more effective and you increase your scope, you also increase the likelihood that you will incinerate yourself.
So we are very careful about correctness of code.
[…] And I think that the primary way that we think about correctness, that we try and get to correctness, is by trying to write code that’s understandable, and having strong practices of reading that code.
— Yaron Minsky (Jane Street CTO), Effective ML
Just as many people talk about TDD, I believe payments engineers should be talking about Code Review Driven Development.
As in thinking about code from the point of view of the person that’s meant to review it. Especially in a financial setting, because there, it’s not the happy path that matters. It’s the weird scenarios, what the industry calls fat tails, that matter.
Weird cases don’t happen often. But they happen more often than you think.
Payments engineers must have code that they understand, deeply. Not just how it normally behaves, but also its internals, its limits, and its nuances.
That’s something you simply can’t get just by writing. You have to read code, a lot of it.
And we may be entering an era in software engineering where reviewing code may be a more important skill than actually writing.
But the steps to do that effectively aren’t that hard.
You first check the tests to gain context
Then you check the issue description to mitigate any gaps with the requirements
Next, you come up with a baseline on how you would’ve done it
And finally you compare what’s done against your expectations
Bookmark this for future reference. Chances are there’s a pull request waiting for you today.
Now you know how to review it well.
This has been The Payments Engineer Playbook. I’ll see you next week.