Welcome back to the Deep Dive. Today, we're opening up the hood on Code Review. It's a practice that's just fundamental to modern software development, isn't it? But you know, it often feels like this really intense, super specific kind of conversation you have at work. Oh absolutely. It's probably the single most effective safety mechanism any software team has. And we're not just talking like catching typos. We're talking about the real, medium and long term health of a
systems code base. Maintainability. Yeah. And it's not niche anymore, is it? This is pretty much everywhere. It really is. It's standard practice. I mean, the data backs that up quite strongly. Right, you mentioned some data, the Stack Overflow survey. Yeah exactly. Back in 2019 they surveyed something like over 70,000 developers and the results were pretty start only about 23.6%, so less than 1/4 said they didn't use code review in their day-to-day work.
Wow, so more than 3/4 of developers rely on this? It's huge. It is. It shows how critical this feedback loop is considered across the industry. OK, that sets the stage perfectly. So our mission today then is to do a deep dive into the source material.
We looked at specifically a 14 code review by Marco, Tulio Valente and Aline Torres. We want to pull out the the core mechanics, the reasons behind it, the motivations, and maybe most importantly, the practical best practices for both sides, right reviewers and authors. That sounds like a plan. Where should we start the basic process? Yeah, let's start there. Yeah, fundamentally, the concept
seems simple enough. A developer writes some code and before it gets merged into the main system, at least one other developer, the reviewer, has to inspect it. Right. And that inspection kicks off this well, this established dialogue. It's got a structure. It usually starts when the reviewer adds comments to the code changes. Those comments can be about anything. Really. Pretty much.
They might be asking for clarification, like why did you do it this way, or suggesting improvements may be a better way to structure something. Or, you know, the big one, identifying an actual bug. OK, so the review happens, comments are made, then the interaction really starts. The author gets that feedback. Exactly, and they have to respond professionally. There are basically 2 paths.
Either they take the feedback on board, modify their code based on the suggestions, or they reply and justify their original approach. Maybe there's a good reason it was done that way. It's a conversation. A negotiation, Almost. Kind of. And that back and forth continues until the reviewer is satisfied they give their approval, and only then does the code actually get integrated. And we see this happening all the time in tools like GitHub, right?
With pull requests or PRS. Yeah, PRS are the common mechanism, and our sources had a great real world example from APR on the Microsoft Disc Code Project Big. Project. Oh yeah, huge. What does that example show? It really highlighted the, let's say, the delicate balance you need. In this conversation, the reviewer did something interesting. They started with praise, congratulated the author, said the solution was elegant. That's important, isn't it? Starting positive.
Crucial builds trust, makes the critique easier to swallow. Because then they switched gears. OK, they pointed out several issues. Very specifically, things like an internal design problem exposing a method that shouldn't be public. Technical stuff. Right, but also smaller things like layout issues, colors, tab alignment, code hygiene. OK, so big picture in small details. Exactly, and then this was key when they found an actual bug.
They didn't just say this is broken, they attached a video showing how to reproduce it. Oh nice. That's helpful. Super helpful. So you see that mix praise, specific critique covering different levels and clear evidence for bugs. It's it's serious technical feedback, but delivered respectfully. OK, that nails down the how pretty well, so let's move to the why. Why do teams spend so much time on this? I mean, finding bugs is the obvious one, right?
That's usually the first thing people think of. Yeah. And it came out on top in that big 2013 study by the Kelly and Bird. Right, the Microsoft one. Nearly 900 developers and testers surveyed. They asked them straight up, what are your main motivations for doing code review? And number one was finding bugs. No surprise. There no surprise at all. It's the immediate, tangible benefit, catching problems before they hit production. But the study found other reasons too, didn't it?
Deeper ones. It did, and these are arguably where the like the long term strategic value really comes in. They identified three other big ones. First, just improving the code overall, making it cleaner, easier to maintain later. OK, makes sense. Second, proposing alternative solutions. Maybe there's a more efficient algorithm or design pattern that fits better? Right. Different perspectives. Exactly. And 3rd and this one I think is huge is knowledge sharing.
OK, let's unpack that knowledge sharing. How does that work in review? Well, it flows both ways. You see, the author learns from the reviewer. Maybe the reviewer has more experience or knows that part of the system better. But the reviewer also learns they have to understand the new code. Maybe it touches a part of the system they weren't familiar with, or uses a technique they haven't seen. So it stops knowledge getting siloed like preventing those knowledge islands people talk about.
Precisely. Those islands are so dangerous in software projects. Yeah, where only one person understands how something critical works. And if that person leaves or is unavailable, you're stuck. Major risk often called the bus factor, right? How many people need to get hit by a bus before the project stalls? Grim. But yeah, code review directly fights that. It forces knowledge to be shared. Multiple people see the code, understand the logic.
It makes the whole team and the project more resilient. OK, that makes the strategic value really clear. Finding bugs is just the start. It's about quality, alternatives and, crucially, spreading knowledge. Exactly. All right, so now let's imagine I'm the reviewer. I've got APR in front of me. What specifically should I be looking for? What's on the the checklist beyond just? Does it work? It's a pretty comprehensive checklist actually. You start with the basics, of
course. Bugs and functionality does it do what it claims to do. But you need to go deeper. Think about concurrency problems, race conditions, deadlocks, things that might only show up under load. Tricky stuff. Yeah, and error handling. Are exceptions caught properly? Are error messages useful? That sort of thing. OK, so functional correctness and robustness. What about the codes structure, The design? Hugely important for the long term. You're looking for code that's,
well, overly complex. Code that's hard to read, hard to understand. That's a major red flag for future maintenance. Complexity kills. It really does. Also, does it violate any established design principles the team follows? SOLID principles maybe? Or does it clash with the overall system architecture? You need to protect that architecture. OK, design principles, architecture. What about efficiency? Performance. Yeah, that's on the list too.
Is the author using fundamentally inefficient algorithms or data structures Like scanning a huge list repeatedly when a hash map would be instant, right? Obvious performance bottlenecks. Obvious ones, yes, but you also need to watch out for the opposite premature optimization. Optimizing code that doesn't need it. Exactly. Spending ages making some rarely used function a tiny bit faster when that time could be spent on features or fixing real bottlenecks. It's a balance. Got it.
And then there's the the hygiene factors, the small stuff. Yeah, code hygiene. This is where things like code smells come in. Code smells for listeners. That's not a literal bug, right? It's more like a warning sign. Precisely. It's a symptom in the code that suggests A deeper problem might exist in the design or structure. Things like methods that are way too long, or lots of duplicated code, or classes doing too many
different things. Okay, this category also covers the basics like are they following the teams naming conventions? Is the indentation and layout consistent? Readability matters. Absolutely. And finally, what about the sort of safety net stuff? Tests, documentation, security. Essential checks? Is there adequate test coverage, or maybe no tests at all? Is complex logic properly documented with comments and crucially, security?
Are there potential vulnerabilities, improper handling of sensitive data, privacy issues, and even things like are they using third party libraries or APIs correctly, or are they pulling in some unauthorized library that could cause licensing headaches or security risks later? It's a lot. The reviewer is really acting as a gatekeeper for overall system health. That's a good way to put it, A holistic gatekeeper. Which brings us to the human element.
Yeah, because reviewing based on that huge checklist, well, it requires tact, doesn't it? If the feedback isn't delivered well, the whole process can break down. Completely. This is where soft skills become just as important as technical skills. Michaela Greylers work analyzing hundreds of GitHub comments gave some great pointers here. Okay. What were the key takeaways for reviewers? First rule is probably restraint, especially with
alternatives. Only suggest a different way if it's clearly objectively better, not just because you would have written it differently. That's a tough one, resisting the urge to impose personal style. It is. You have to avoid purely subjective comments, like arguing about variable names, unless they're genuinely confusing or violate conventions. Focus on substance. And professionalism in tone is paramount, I assume. Absolutely no offensive language, no sarcasm, no irony.
Keep it professional and keep the comments strictly about the code. No personal stuff, obviously. OK, so maintain professionalism. What about specific tactics to make the feedback constructive? The sources mentioned framing things as questions. Yes, that's a great technique. It softens the critique immediately. Instead of saying this variable is useless, you ask, hey, is this variable actually used anywhere? Maybe we can remove it? Right, opens a dialogue instead
of issuing a command. Exactly. And another powerful one is using inclusive language we instead of you. How does that help? It reinforces that you're a team working together on the same code. So instead of you should make this private track. Could we make this attribute private? Might encapsulate things better. Small change, big difference in feel. Yeah, I can see that more collaborative. Definitely also back up your comments if you suggest a change, especially a non obvious one.
Provide a reason linked to documentation, internal style guides, external articles, justify. It and that justification is even more important for junior developers, right? Yeah, It becomes a teaching moment. Absolutely don't just tell a junior dev. Use a hash map here, not an array list. Explain why for this kind of data access, a hash map gives us much faster lookups when the data set gets large. Teach the reasoning. OK, justify suggestions. What else?
Positive feedback. Yes, don't forget praise. If the code is well written or the tests are really thorough, say so. A simple nice work on the clear test setup made review much easier. Goes a long way. It balances the critique. Guilds Goodwill. Totally. And finally, humility. As a reviewer, you will be wrong. Sometimes you'll misunderstand something. When the author corrects you just acknowledge it gracefully. OK, I see what you mean now. Thanks for explaining. No need to get defensive.
And what about disagreements when you just can't agree through comments? That's when you might need a synchronous meeting like a quick call, but that should be rare, really reserved for significant disagreements, or when you're just not converging on approval via comments. Make it the exception. OK, that's a great set of guidelines for the reviewer. Now let's flip the coin. What about the author? The person receiving all the scrutiny? What's the advice for them?
The first thing for the author is mindset. Be professional, be polite, but crucially understand that code review isn't like a test of your overall programming skill. Right, it's about the specific code submitted. Exactly. Don't take the feedback personally. It's about making this piece of code Better Together. OK, mindset is key. What about practical tactics for the author to make the process
smoother? There's 1 golden rule, maybe the most important piece of advice for authors who want good fast reviews, which is submit small pool requests, small PRS, ah. Keep the changes focused and limited in size. Why is that so critical? Because reviewers are human. The authors of Software Engineering at Google are really clear on this. They recommend APR should ideally be no more than say 200 lines of code. 200 lines that sounds manageable to review properly.
It is. It lets the reviewer focus, understand the context and actually find subtle issues. When PRS get huge, quality just plummets. Yeah, there's that great quote. Wasn't there something about reviewing 20 lines versus 500? Oh yeah, the tweet cited in the sources. It sums it up perfectly. Ask a programmer to review 20 lines of code. They'll find 7 issues. Ask them to review 500 lines and they'll find 0 issues. Yeah, it feels true, though reviewer fatigue is real. It absolutely is.
If you dump 1000 lines on someone, they're just going to skim it. They might catch obvious stuff, but the tricky bugs, the design flaws, they'll likely slip right through. Small PRS force focus. So small PRS mean better quality reviews, faster integration, and ultimately less risk going into the main code base. Couldn't have said it better myself. And beyond size, authors also need to write clear PR descriptions, explain what the change does and why it's needed,
give the reviewer context. And be prepared to justify decisions if asked, just like they expect reviewers to justify suggestions. Exactly. It's a two way St. of professional justification and explanation. OK. So if we zoom back out, the big picture here is that code review isn't just a gatekeeping step, it's this really vital
collaborative dialogue. It ensures the code itself is robust, yes, but it also drives that crucial knowledge sharing, keeps the team aligned, and ultimately keeps the whole system healthy. But it relies on everyone approaching it with professionalism intact. That's it in a nutshell. It's really the engine room of high quality sustainable software development. Get it right and so many other things fall into place. Get it wrong and you're storing up problems for the future well.
Put a critical process that needs care and attention from everyone involved. Thank you for joining us for this deep dive.