Wednesday, August 1, 2012

Code Review Best Practices - Review Everything

Welcome to the second part in a series on best practices for code reviews. Last time, I explained why I believe it's important for everyone on the team to chime in on code reviews -- rather than having a single gatekeeper, it's important for everyone to share that responsibility.

Today I'm discussing the flip side of the coin -- everyone's code should be subject to review. I believe even the technical leads should submit their code for review. Of course, it's tempting to let their code slip by because, hey, if they don't write code that passes the team's standards, then who the heck does?!

But there are actually very good reasons to subject even the technical lead's code to review. Here are three of them:

Experienced != Flawless

Sure, the tech lead will probably style his code according to spec -- things like indentation and placement of curly braces. But there are tons of details that even the most experienced developer can miss.

  • Did you introduce an off-by-one error?
  • Did you accidentally create a global variable by forgetting to locally scope it?
  • Did you forget to specify a service to be a singleton in your dependency injection configuration?
  • Did you accidentally leave a debugging statement?
  • Did you use a variable name that others might not understand clearly?

Although older and wiser, technical leads aren't perfect. Details can elude anyone. And every developer is subject to tunnel vision. We need others to check our blind spots.

Personally, I want others to look over my code, because I simply can't see everything.

Less bureaucracy. More teamwork.

It's easy for technical leads and architects to be regarded as ivory tower elitists who keep others from "just getting it done." By subjecting their code to review, you foster much more of a team spirit. It makes a statement that says "we're in this together."

Remember, everyone on the team is responsible for the quality of the code. Technical leads, however, are accountable for the quality of the code. For that reason, they do get the final call on decisions that need to be made. But, by putting their code up for review, we encourage everyone to take ownership of the project.

Share the knowledge.

If you play an instrument, you know that one of the best ways to improve is to practice with people who are better than you. Somehow, it gives you a rung to latch onto, in order to pull yourself higher. Coding is the same way -- I've found that I can learn a lot just by studying code of people who are excellent programmers.

As a tech lead, if you withhold your code from review, you rob junior developers of the opportunity to learn from you. So let them review it. Let them ask questions. Let them challenge you on things. Remember, you're not just developing applications, you're developing programmers.

So, no more free passes. Everyone puts their code up for review. Sure, it might mean humbling yourself. But it's actually quite liberating to admit that you're not perfect, and that we're all in this together!

No comments:

Post a Comment

Profile Picture
Dave Leeds
My Hobbies:
  • Programming
  • Cartooning
  • Music Writing
Full Profile