Code review of randomness

Apr 10, 2012

Last week, prior to a weekly meeting, one of my team members had the idea to
take a few minutes before the meeting to review some code. We already do code
reviews prior to deployments, but we’ve been wanting to improve the quality
of the code reviews… we wanted to really have good dialog about code quality
and be able to share that discussion with the whole team.

Code reviews… but random

So we were all on board with a more fully featured code review… but whose
code would we choose? We weren’t exactly getting volunteers left and right to
offer up their code… probably because they knew that everyone else would
find some reason to skewer it. After all, no code is perfect… there is
almost always room for improvement.

Another problem with volunteered code is that it would typically be code that
had been seen recently. New code tends to be of higher quality than legacy
code; however, we have plenty of legacy code that could be greatly improved by
having the team take just 10 minutes to look over it.

The solution was a PowerShell script that I whipped together in about 15
minutes. Here it is:

As you can see, it is pretty basic. Most of the complexity involves excluding
Designer or generated files. Once the script determines a random file, it
opens it in a text editor for the team to view. From there, we began looking
at it as a team and calling out issues… “we should use var here,” “can we
rename that variable?,” “I think we need to dispose of that object,” etc. In a
short 15 minutes, we as a team were able to find all sorts of minor issues
with the code.

Team Learning, Team Dialog, and Decreasing Technical Debt

By reviewing the code together, it allowed us to share the learning about what
we as a team thought made for quality code. Often while pair programming,
these discussions come up, but they were only between 2 or 3 at most… by
doing this together, we could all benefit from the discussion.

Another benefit is that it forces us to become familiar with random parts of
our system. I’m confident that almost every development team has some aspect of
the “Train (or bus) Problem” as I call it… if someone on your team were hit
by a train (or bus), could the rest of the team pick up without them? If not,
then there is a potential liability there. By reviewing code as a group, the
team can become familiar with the code together.

From a technical debt perspective, we now have another tool in our arsenal
that we can use to slowly improve code. Ideally, if we continue improving our
core code in this way, in six months or so we’ll have trouble finding issues.
We might have to go after specific sections of our code to review at that
point. Either way, we’ve got something started that seems to be very
beneficial.