The Ends do not Justify the Means: The Necessity of Code Review

Hopefully this post does not sound like just a rant as I admit that this post has come up from my frustrations from using other people’s work, but I think this is particularly symptomatic of areas where there are not always IT experts, as is the case with many libraries. I also do not boast that I always know what I’m doing, but that’s why I think it’s invaluable to have some kind of code review process.

Making it Simple

As a general rule, better to make things as simple as possible rather than using complex methods if they both achieve the same things. I’ve been told that simple and clean code is the best kind of code.

There may be efficiency concerns involved, but then that seems to be left behind in the cases I’m talking about anyway. (Case in point, our website had 5+ CSS files, not all which had clear purposes, many of which were overwriting each other’s classes, which I had to rework into two files.)

Document and Comment

While some of the simple things might be obvious and self-explanatory, I don’t think I’ve ever seen anything over documented/commented in the ‘real world’ (as opposed to school work), so more is (generally) better. I think it’s doubly important where the creation of a product/tool might be outsourced, but the maintenance of it is done in house (as frequently seems to happen), that the code can be understood by a new/beginner programmer-type person.

Too often I have looked at code recently and thought “what does this do?” or “how many pages does it affect?” While I admit that it’s not always easy to know what or how much needs to be documented, because you don’t always know who will look at it in the future and things you write yourself tend to make sense, but do at least a minimal amount of documentation and commenting.

On a side note, do not name files or functions after yourself (one former student’s name is now unforgettable for that reason and this was created months before I started).

Get Feedback

Ideally, there is someone who is an actual programmer on your team or in your unit who you can ask to take at least a quick look over what you’ve done and give you feedback on whether you’ve done it correctly. If there’s no one ‘inside’, consider asking someone on the ‘outside’.

Within a team or unit, if there is more than one programmer-savvy person, consider establishing a Review/Feedback step in the process as you typically see before code commits (if this doesn’t already exist of course).

If you really can’t get feedback from someone else, then much like editing your own writing, step back from it (for, preferably, a couple of days) and look at it with fresh eyes and ask yourself some of the important questions, but namely:

would someone else be able to step in and easily understand what your code does?

Supervising Students

I believe that all the above points apply when working with students too. It’s okay if you’re not a programmer. At least take a brief look at the code, see if commenting is done, ask about the general process/framework, ask if they think a beginner programmer would be able to understand their code easily, etc. If you ask, it will at least make him/her think about it!

My Point

The end product does not mean it is what you want. Other people also need to be able to maintain and customize/modify. This all seems so obvious, but I think it bears reminding sometimes.