The topic of code reviews has always caused much debate in many software shops where I have worked.
I am often asked what my opinion of doing code reviews is. My answer might surprise you.
I don’t like them. I prefer not to do them.
What?
Let me be more specific. I don’t like code reviews that happen after the work is already done.
I do like code reviews that happen continuously as the work is being done. This is called paired programming. (This prezi presentation might also be helpful.)
Paired programming = continuous code review?
Yes. That is exactly what I am saying.
Think about it for a second. Can you really have a better, more honest, and more interactive code review than paired programming provides? If two developers are working jointly on a portion of code, it is being reviewed as it is being written.
I’m going to divert a bit to talk about some of the common problems that happen in code reviews, then I’m going to talk about how paired programming solves them.
Code review problems
#1. Not telling the truth
There really is no value in a code review where no one says what they really think is wrong with the code and instead picks out some little non-offensive thing like a naming convention that wasn’t followed.
All too often code reviews are dominated by a fear of insulting the person’s code that is being reviewed. This kind of weak code review provides no benefit. I would go so far as to say it is reinforcing the practice of writing bad code because bad code is rubber stamped.
#2. Ego
To have a successful code review everyone has to check their ego at the door. This rarely happens. Code is something that has been creatively designed; not too many people can take a critique of their labor, even fewer of their art.
Without a spirit of humility and openness, code reviews don’t provide any value. It is one thing to point out a problem, it is another thing to do something about it.
#3. Wrong focus
One of the most common errors of code reviews is paying attention to formatting, coding style, or naming conventions. Those topics don’t belong in a code review at all.
Tools can automatically format the code and statically analyze problems with coding style or naming conventions. It is a complete and total waste of time to have a human reviewer review any of those things.
Seriously, if someone comes into a code review and they haven’t run the auto code formatter on their code and they haven’t run the static analysis tools and fixed the problems found there, don’t be rude, just tell them nicely that their code is not ready for review.
Side note here: There is often debate on whether we should enforce code formatting, or enforce coding style, or enforce naming conventions. YES! YES! YES! Consistency is hugely important! Code is read more than written and consistent code is easier to read. Have this discussion once, put together the automated tools to automatically format the code, and automatically check the requirements.
Don’t spend months making up your special coding standards document. Use some rule sets from any number of static analysis tools and start enforcing them. Make them part of your build, make check-ins fail for formatting problems, make your IDE auto-format on save. Deciding the perfect placement of curly brackets and whitespace is not nearly as important as being consistent. Just pick something and enforce it.
Code reviews should focus on design, structure, and understandability of the code. Code reviews should never ever ever ever ever ever ever focus on anything that can be automatically checked or enforced… ever!
#4. Rush & Inconsistency
Teams rarely have or make time for code reviews. If they do, it is inconsistent and not applied to all code, which causes the value to be lost. If you have a huge fence around a perimeter yet a large hole where people can just crawl in, the fence is worthless.
There is no point doing a code review where the code is not reviewed in depth. Just scanning over the code and picking on a few variable names here and there does no good. If you're not prepared to fully immerse yourself into the code being reviewed, and dive deep down into understanding exactly what it is doing, don’t bother.
Paired programming to the rescue
Paired programming is like just-in-time code reviews. It solves many of these issues by having a common goal and creating a joint ownership in the code.
As you pair up with different members of your team, you are likely to be more honest, be less egotistical, focus on the problem not the conventions, and delve deeply into the code.
It is no secret that my preferred modus operandi is no code review, instead paired programming, with everyone monitoring commits to source control, and can flag something and have a discussion if they see something troubling.
What if paired programming is not an option?
Well, try to make it one. Paired programming is a great investment.
But, if you are not doing paired programming, you absolutely should be doing code reviews. I might have extolled some of the problems with code reviews and I advocate paired programming, but let’s not mince words here.
Pair programming is a better way of doing continuous or just-in-time code reviews. Not doing any form of code reviews is just plain stupid.
If you had a construction team building your house or your 20 million dollar sky scraper and they told you that they were not doing engineering reviews of the work that was done, you would probably fire that construction company on the spot. Building software is several levels of complexity higher than that of most physical construction projects. If you call yourself an engineer and you are not subjecting your work to regular review, you are just an amateur playing “engineer.”
So, if you must do the old-fashioned code review, here are some tips I found useful:
- Encourage people to be honest by doing some exercises where everyone reviews some code that wasn’t written by anyone on your team. Encourage people to be as prejudice as possible when reviewing the code.
- Set your expectations. Code reviews are not an approval process, they are an improvement process. Make sure the team is aware of this. Code reviews are a place you take your code to make it better, and to better your skills. Foster that kind of thinking.
- Automate all of the nit-picky stuff and set coding standards, formatting standards, and style standards. Decide what they will be together, or have your experienced people do it, but get it done, make conformance non-optional, and make it automated.
- Develop consistent standards and practice about doing code reviews. Set aside enough time to delve deeply into the code. Set and enforce standards of when and how code is reviewed.
- Lead by example. Proactively ask members of your team to look at your code and tell you how to improve it. Ask if this can be done a better way. Show a genuine interest in feedback and improvement, this attitude will catch on.