Your Code Reeks

Drudging through the each line of fluff; var i, .length, foreach.

“How in the world does this code even work?”

We’ve all been there.

As a code review artist myself, I run into problems daily that make absolutely no sense upon initial understanding. Frustration leads to anger, anger leads to retaliation, and then the pull-request has been denied with prejudice. The problem with the code however; is you. When reading code developed by another programmer, a few scenarios play out in the reviewers mind.

The first is the ‘hacker’ syndrome. For any nerd, we’ve been bred to believe that when reading code not understood, but is semantically accurate and well spaced, it must have been written by an amazing programmer. From that point forward, reading the code is assumed a learning process and the code written, gold. The polar opposite effect is the ‘n00b’ syndrome. Easy to see how this plays out. The reader notices on something that annoys them, or is found sloppy and against ‘best practices’ (I define these extremely loosely) and everything the programmer writes become goto and spagetti. Sometimes the code reviewer respects the fellow programmer, and gives the benefit of the doubt, but can’t pass some of the obvious mistakes. Other times, the reviewer simply doesn’t care or doesn’t find the time to involve themselves into the details of the code changes.

In each of the cases outlined, it is the code reviewer whom is at fault. That can, and should be where correction is taken. It’s hard to review code each day and it is at times very monotonous. I’m of the mindset that when doing a code review, the time put in to correct mistakes will at most equal the time that would have been put in to apply a patch or fix a bug that slipped through into the user’s hands. Here are few things to keep in mind when reviewing code.

Obvious bugs are caused by an undefined spec being developed by an unaware developer.

§ given a well specified action to take, nearly any developer will produce code that will solve that action in the expected behavior.

As the code reviewer, your job isn’t to teach, it’s to learn.

§ you can’t review code if you decide to tell other coders how they should write a loop efficiently, rather than learn why they are using a for loop to begin with. Most of the time, there are valid reasons to the solution the coder has written.

§ if you know that the code is incorrect; prove it. Write a test case that fails, run a load balance or check the performance using your implementation against it. If you can’t solve the problem or prove that it is a problem, then the coder did their job and you learned something. If you can prove it, the other coder may learn something, but so did you; as the act of proving it has given you that much more knowledge of the problem they were attempting to solve.

Rushing to a judgement will blind you to the problem being solved.

§ make sure you have a full understanding of what you are reading before questioning the code your are reviewing. Not having a clear picture may cause you to look foolish; and no one wants that.

Stick to what you know, and know what you know, everytime you need to know it (intentionally makes little sense)

§ it’s hard to say, ‘I’m not sure what this does’ but that in itself can be the most wise review decision you can make. If the developer cannot describe it, or understand it either, then it’s likely broken or not solving a real problem.

§ you need to keep a good log of the things you do not approve of. In Ecmascript, things like global variables, variable hoisting, proper use of ‘this’, are things you should always deny and make no excpetions when you do.

Be responsible, be respectable

§ no one likes an unfair bigot. Follow your own rules and make well intended notes upon rejection of code.

§ ”your code is s**t” is the absolute worst response on any level to any code review. If this is the response you want to give, then you no longer understand your job and you are certainly the problem, not the code.

Reviewing code is fun and highly encouraged. Make it a learning experience and communicate truthfully and the code you review will greatly benefit.