Reviewing review

Code review is something that nearly all software developers consider to be a good thing. Though frequently when I’m doing it, I feel out of my depth.  This is usually when I’m learning the technology or codebase.  And recently I’ve been reviewing not just an environment in which I am comfortable (a desktop application) but also a web application, which covers the whole stack right from database, webservice, client and javascript-based webpage.  My experience to date has been in embedded software, desktop configuration software, a network-based build server and a smattering of almost static html, so I’m feeling somewhat out of my depth when asked to cast an eye over a SQL stored procedure.

I kind of understand SQL.  I can create a database and do a couple of queries on it.  Joins are something I’d only previously done implicitly.  Sure, I can read the code and see if something’s obviously wrong, but I’m only catching errors that a strong cup of coffee would usually fix (unless applied directly to the keyboard).  I’m currently questioning the notion that any second pair of eyes looking at something is necessarily a good thing. I’m not questioning that the right pair of eyes would improve the code, I take that as a given.

Which naturally leads to a few questions:
a) Is code improvement the only benefit of code review?
b) How does one become the right pair of eyes in the first place?
c) How do you diseminate the ability for others in the team to be a right pair of eyes, such that the team is productive and not artificially bottle-necked on one or two people?

The main times when I feel like I’ve been contributing to a code review are when I was either the original author of much of the code, or have made significant changes in the area in question.  Other times, when I’m vaguely aware of the codebase or even still very much learning the technology (like MS SQL Server, for example) I feel like I’m actually doing a disservice to whomever is getting the code reviewed.

a) No. The reviewer also benefits from reading the code. Reading code is an important skill that is highly underrated. It’s like reading in a second language – it’s fairly easy to understand the black and white portions of code, but the subtle nuances and tonal inflection can only be ascertained with deep understanding. Reading code is the only way one will understand it well enough to slot an addition neatly into the existing functionality, rather than bolt another fragile pimple onto the side of the pumpkin. The issue here is that neither of these *also* benefits the reviewee.

b) Presumably by reading, playing with, altering and understanding the code as a whole. Yes, there’s no real secret here – the answer is hard work. Does this also involve reviewing code? Yes! Reviewing code is reading the code, but thinking about it differently.

c) Now that one’s really difficult. The best way that I know of is probably Rubber Duck Debugging. The original person who researched the start-up code or technology or has the experience of working with it before completes a task and then walks someone else through it. Describing it as review in order to fit it into your Department of Defence Definition of Done, can be handy, but most times I do this, I talk myself through a bug or design flaw.

There is more than just one approach to code review and choosing the right one is largely dependent on the individual and the technology. It’s tempting to also add in the relative skill and experience of the reviewee here, but I’m loathe to do that as we’re all human, sometimes make dumb mistakes or just over complicate things. There is also more than one benefit from code review. Whether it’s learning a cool new idiom, increasing one’s understanding of the system, or even the most obvious of evaluating the style, design and implementation of a piece of functionality. ” Perfection is achieved, not when there is nothing more to add, but when there is nothing left to take away.” [Antoine de Saint-Exupery French writer (1900 – 1944).]

In the end, I guess what I’m saying is that reviewing stuff you don’t understand is hard and you feel like nothing is being achieved. But it’s a way of getting your head into a code space without directly damaging it. The reviewee isn’t benefiting a whole lot, unless they’re walking you through it (which if you’re that new to the code, they probably should be anyway).