Life in Code Review (or what it takes to build a quality product) - Part 1

Throughout my career, I have been asked the same question: “What are you looking for when you review someone’s code?” A few things always immediately pop into my head, such as inconsistent code formatting or over commenting. But, truly, code review is a vast field, with many interesting (and sometimes nonsensical) moments that can keep anyone up at night.

To preface this, I can honestly say I still make mistakes (to this very day) when I write code and prefer to have a coworker review my code. No one should be absolved from a code review. There were instances in my career when I developed software components that now prompt the reaction of “What the heck was I thinking?” Unfortunately, in those particular instances, my code did not receive a proper review.

Prior to even starting code review, it is important to steer teams toward sound design decisions. A proper code review isn’t something that holds the production line, it actually sends back the product to be completely redesigned and redeveloped.

Over-engineering

Symptoms of an over-engineered project include over-scoped or misunderstood requirements provided by the users, producers, and/or business unit. It could also emerge when the developer or architect actually tries to over-engineer. One popular term for this issue is this “RDD”, short for Resume Driven Development. Determining if a project is over-engineered can be difficult. Many of us have seen code or projects and, with one glance, decided that it was over-engineered. Most of the time, the blame falls on poor user interface, user requirements, or other members of the team. Other times, a part of the project was engineered correctly but another component wasn’t, thus giving the perception that the whole project is over-engineered.

As a whole, promoting a collegiate culture among peers will help the team hold itself accountable. Enact the following specific criteria for better design decisions:

• Understand what the users of the system need (not want) to ensure there is no additional functionality. Basically, focus on what the system will do and not what it may do.
• Ask yourself:
  • Can a mid-level engineer who is unfamiliar with the system fix any bugs? In turn, how quickly can we get it to market?
  • Can we build something minimally viable and iterate over the feature set?
  • Are there premature optimizations?

If everyone on the project is thinking along those lines, including the developers, then there is a higher likelihood that the project will be completed on time. Agile methodologies can help too, and so can providing your junior and mid-level engineers with basic bullet point details on what is changing will bring them into the right mindset..

Over-commenting

I am an advocate for minimal commenting. There are many occasions when comments focus on the minutiae of the code. This practice inevitably slows down the next person that needs to refactor the code as well as, in many cases, decreases code readability and propagates meaningless, stale comments throughout the codebase. Unless you are building an outside-facing API that needs to be integrated, most methods do not need to describe what the parameters do or much of anything else for that matter.

Someone once told me, “A developer saying “I’ll do it later”, means he’ll never do it.” In many cases, if something is necessary for the system to function, just get it done. However, if it is something that isn’t essential, then why even write a to-do? Functionality might be ripped out, refactored, or changed completely during the evolution of the code, so finalize the to-do prior to committing the code.

Self-Documenting

I’m a firm believer in a verbose definition of coding variables. As long as developers choose their parameter, variable, function, method or class names carefully, then the code should be self-documenting. For instance, if I need to store the year Media Temple launched our managed WordPress solution for temporary calculations, I would first create a variable ‘yearMTWPLaunched’, assign ‘2015’ and use that new variable, instead of just using ‘2015’ in the calculation. By assigning a well-defined variable to a number, I am removing ambiguity as to what that number means.

In conclusion, reviews are crucial but, in order to have a quality product, code testing, code analysis, and code execution are just as important. We’ll tackle these topics in part 2 – stay tuned!