Guess what? A conversation I was having on Twitter got me excited enough to blog again. Perhaps reading Twitter isn't such a depressing waste of time after all.
The problem with code reviews is that it is too easy to end to end up with code targeted at the lowest common denominator of the ability of the reviewers.
— Ihe Onwuka (@mazihe) October 21, 2020
(Usual caveats here: if someone's Tweet inspires me to rant, it doesn't mean I disagree (or agree) with that person, it usually means it's just sparked a cascade of thoughts in my head.)
This led me to one of the most frequently-asked-questions:
Can junior developers review code?
There's a few variations on the same theme:
- Can people new to the code/application/business review code?
- Does it have to the most experienced people on the team who perform the role of the reviewer?
The unspoken assumption here is that junior/new/inexperienced team members can't add any value to a review because they're not good enough.
Which is nonsense.
I mean, firstly it makes me want to rant long and hard about how this industry is terrible at training up juniors/apprentices, about how we lazily rely on bootcamps and universities to create junior developers at the standard we require of them, when in fact that's not their job, it's our job as "the industry" to get our juniors to "industry standard", whatever that means.
It also makes me mad because we seem to always be looking for ways to tell developers how bad they are, how they haven't made it yet, how they have so much more to learn, how they're not worthy. When any truly senior technical person will be the first to tell you: they don't know anything; there are no right answers; this stuff is hard.
Anyway. Deep breath. Relax, Trisha, and get to the point:
Junior developers are The Best People To Review Code.
It goes back to the whole Readable Code thing. If the code under review requires a senior person to understand it, it's not Readable Code. If I write something that a junior can't understand, I did not write something that's maintainable. If we have juniors on the team, the junior needs to be able to work on that team. Which means they need to be able to understand the code. They can't write code if they don't understand the code they're trying to change or add to. They can't write code if they don't understand what the tests do.
Juniors are your best canaries. They can go into a code review and ask the stupid questions: Why is it called this? What does this do? Why are we testing this bit? Where is the test for this edge case? What happens if...?
If your code doesn't answer those questions, it needs modifying. Or documenting, or whatever approach you choose to share knowledge. Code review can absolutely be one of those ways to share that knowledge. When a new member to the team has done 3 or 4 reviews and sees the same patterns appearing, they'll begin to see that's one of the common patterns for doing things in this team. When a junior sees all the code reviews include automated tests for happy and unhappy paths, they understand that's a requirement for the story to be "code complete".
When a junior reviews code, everyone wins:
- The code's readability improves
- The junior learns how things are done on the team
- The team understand more of the inbuilt assumptions that might either need to be explained, externalised somewhere, or even challenged and removed.
What if there's some stuff that Only Senior People Know?
Yes good point. A junior isn't going to catch gotchas that people who've been working on the application for ages just know. A person new to programming on the team might not know that the preference is (for example) for using the newest features in Java where available, or even what these are. A developer new to this business domain won't necessarily know about regulatory requirements that need to be met when certain features are implemented.
This is fine. This just means code reviews (if you do them) should be a team effort: you all have your strengths and weaknesses, and as a team you compliment each other.
What's great about this is it also solves (or reduces) one of the other Great Code Review Problems. You know the one: your code takes aaaaaaages to go to production because the seniors reviewing the code are the bottleneck. Plenty of organisations have a policy of All Code Must Be Reviewed By The Senior Architect Before It Goes To Production (something I call a Gateway review). The poor senior architect is drowning under reviewing everyone's code, and barely has time to do her/his own job. If phase one of the code review is to give it to a couple of less senior people first, they can review it for understandability/readability, does it seem to do the thing the original requirement/bug requested, are the tests understandable/testing sensible things. Once the "is this code understandable and maintainable?" check has passed, then the Senior Gatekeeper only has to check it for the stuff that only they know: does this have some security implication we didn't know, is there some audit missing, is there a weird edge case that hasn't been considered. This should take the Senior Gatekeeper significantly less time than trying to find every problem in every piece of code.
Code Reviews Take A Village
Creating a technical solution goes best when there are a range of people and talents involved. Use your juniors to check your code is understandable by "the lowest common denominator". Use your seniors to check the stuff no-one else knows. Use the whole team so that everyone knows what should normally be part of the code deliverable (standards to be met, tests to be written etc) and so that everyone is at least vaguely aware of the functionality and changes going into the code.
This reminds me of another Twitter conversation I saw that I violently agreed with:
Junior + Junior—fun figuring stuff out together, safely.
Senior + Junior—grow & get feedback on simplicity
Senior + Senior—amazingly productive
Specialist in A + Specialist in B—by our powers combined
Fresh + Familiar—new perspectives & avoid traps
— Benji Weber (@benjiweber) October 10, 2020
Which leads me to my final caveat: Code Reviews are a Good Thing, but honestly if you can do pair programming on your team, it's even better.
PS If you're interested in my take on how to create a code review process that works for you, I've got a presentation about that you can watch. I'm also giving that talk again this coming Tuesday
One Reply to “Who’s “Allowed” To Review Code?”
“Junior developers are The Best People To Review Code.” – This seems great idea, if they can understand and read code means code is really good on readability point of view. Nice read, thx for this awesome post, I have also tweeted from @javarevisited .