Saturday, August 1, 2015

Quick and Dirty Code Reviews

By now, the code review is well understood and respected as the vegetables of healthy programming. And as we know from any meal eaten in haste, the vegetables are often the first thing to be left on the plate.  I've seen this kind of neglect a lot in computer animation production where the code only needs to work until the final frames are rendered.  It's worth arguing that in these cases where code is tactical and only needs to work for a few clients for a brief time, it isn't worth the extra time for code reviews.

This post is a bit of a confession.  I admit I also have neglected my vegetables in the chaos of production.  Now that I've started building realtime VR experiences, however, I need to remind myself that my deliverable is no longer a bunch of rendered images, but an executable.  It makes me nervous to think that my code may be used many years beyond the launch of a project.  Hats off to the programmers of all of the games I've ever played.  So now, even tactical code can have huge ramifications to the end product.  All stages of checkin need code reviews.

But here's the problem.  When code needs to be quick and dirty, for prototyping, firefighting, or rushed deadlines, the rigorous code reviews will inevitably start getting left on the plate.  And so instead of losing the benefits of code review, introspection, shared knowledge, readability, I think the code reviews also need to be quick and dirty.

For me, a big benefit of reviews is having my brain process my code differently when having to explain it verbally to another programmer.  A lot of bugs I've found with a collaborator come from me talking through a section of code and then realizing the code doesn't actually do what I am saying.  This part of a review is easy and doesn't even need another person (see about the Rubber Duck).  But the reason I would prefer another person is that they can ask questions.  Why do it like that?  Is there a simpler way?  What does this section do?  It's in answering these questions or usually because I'm unable to answer these questions that I find code that's more complicated than it needs to be, or code that needs to be rewritten for readability.  For core system code that will live longer than a production, I do appreciate feedback on syntax, line spacing, performance and convention, even when it can feel draconian.  But when I need to checkin a fix to unblock a lighter or animator,  I just want a quick sanity check - something just slightly more interactive than a rubber duck.  A reviewing pair should understand where on the scale from zealot to rubber duck the reviewer should be based on the context of the code to be checked in.

I don't think the rigor of code reviews needs to be consistent across an entire project as long as you find a way to always have those reviews. There's nothing wrong with adding some butter when you're having a hard time eating your broccoli.