How to review code

The first question we need to ask ourselves is what is the goal of our code review?

  • Report all code review issues?
  • Get all feedback incorporated before delivery?
  • Get the developers to follow the same paradigm and make the expected code quality a ritual in their development lives?

Your how should be in consonance with what is your goal. My personal view-point is that the last goal is more feasible than the first two and it will have a more profound impact on the code quality delivered over a long-term.

Till date, I have never seen an application where each and every deviation got reported in time. Despite the best of intentions at the time of delivery, we did find feedback that had to be scoped out too.

Due to these very reasons, the self-belief of the developers in the system will die out sooner than later. If mentally they are accepting that quality in code will not be met, and nothing can be done to avoid it, they will stop trying harder to deliver quality code.

To make good code quality a ritual for your developers you need to embrace the following practices in your team:

Establish a check list of items that will signify acceptable code quality for your team.

  1. Try not to include everything under the sun.
  2. Ideally the checklist should be something that can be printed in one A4 sheet

You may create periodic code quality campaigns in which your team is aware that you will be focussing on some specific code practices with zero tolerance

  1. This technique will be very helpful when you have a lot of pre-existing code that needs to be enhanced and your checklist looks too daunting for your developers as well as making the client ROI too challenging to achieve
  2. Some examples include ensuring all Web pages are compliant with a specific set of HTML standards, JS Lint standards, or, your stored procedures are created using static queries instead of dynamic queries
  3. Whatever campaign is chosen, it should be measurable for both the developers and (if applicable) for the clients.
  4. If the clients are able to appreciate, as in the case of performance boost as a result of static queries in place of dynamic queries, their feedback will encourage your developers even more to embrace the same standards
  5. If the effects are not that visible to your clients, then, it must make the lives of your developers easier. For example, when we targeted improvement in the HTML and JavaScript compliance, we were able to make our sites IE6 and IE8 compatible with lesser efforts and lesser issues reported.

Provide the tools to your developers for improving compliance with best practices. For example, our team has access to Resharper, JS Lint (integrated with Visual Studio), W3C Validator, etc.

  1. Tools like Resharper are great for boosting developer productivity on both the client and server sides

Review relentlessly and as early as possible

  1. Too many times, code is passed on to QA even before it is reviewed, primarily due to budgetary or timeline constraints. In most cases, the overall cost of a stable release is higher due to intermittent issues reported (especially when best coding practices are not part of the developer’s coding rituals)
  2. Once an issue is identified, ensure that it is incorporated by the concerned developer and by all the developers in your team
  3. The key is not how many issues you catch, but, each issue that you catch is stamped out.
  4. Subsequently, start maintaining a list of issues that you generally catch and look out for them in the code first.
  5. Also, build a case of the demerits of the issue and the benefits of the solution and discuss it with your developers
  6. The relentless review pertains to not ignoring the issues already reported in the past until the developer changes habit and begins coding the way expected first time around itself

Last, but not the least, choose your code champions

  1. Select people from the team who will champion the cause of best practices in a particular technology. It is better than expecting the same person to look in all aspects of the code including HTML, CSS, JavaScript, C#, SQL, etc.
  2. Give access to each code champion to do the following
    1.  Look at code in the specified technology across projects
    2. Share common mistakes in periodic lessons learnt meetings
    3. Propose white papers / presentations on how to achieve good quality code and increased productivity via re-usable code snippets / libraries or even suggestion of approaches

In conclusion, the more coding practices you inculcate in the developer’s coding ritual, the lesser time will be required in code review (in relation to those items). Although, code review will always be there as lessons will always be learnt, at least the team will be consistent and meet the standards that are made a focus area. This makes it easier for developers to switch project teams or move from one project to another without losing out on productivity.

Updated on November 20, 2012:

Based on some feedback received on Facebook from my dear friend, Moin, the following seem necessary to be added to this blog.

Further thoughts about raising code quality standards via code review

  • It is far easier moulding entry level programmers towards maintaining coding standards. However, the entry level programmers will always remain a fraction of the pie. A majority of the programmers will always be mid or senior level and many of them would have joined from different firms where the coding standards may differ.
  • For mid and senior level programmers, the same review process can be followed, but as a manager we must be ready to answer tough questions with assertiveness. I have seen this happen in my team and although at times, a bit of effort goes into explaining the rationale, generally, the team support is there
  • Code champions in this article refer to those people who will champion the appropriate usage of the technology, and, have the in-depth understanding to be able to explain their code review to the concerned programmer. Their focus will also be to pick out gems of code snippets and share it across teams as well as facilitate consistent style of programming
  • The code review should not be mixed with code audit. Code audit is done after a phase of coding has been completed. A code audit will definitely let you know what is missing in the code and a lit of issues will be available for the programmer to incorporate. However, it can also lead to programmers becoming defensive about their code. It is better to spend a bit of time with them while they are programming and provide on the spot feedback and focus on assisting the programmer to evolve rather than focus in issue reporting. It will also lessen the pressure on the programmer to incorporate feedback at the end of the coding cycle. A code audit at the end of the coding phase will still be useful in tracking how successful we are in the initiatives taking during coding.

Leadership Wisdom by Robin Sharma states that we all want appreciation and recognition. We tend to do more of what we get genuinely recognized and appreciated for. All thoughts above as well as suggestions are from the perspective of helping our programmers become recognized as success stories rather than being viewed as deficient in some aspect of coding.

 

Advertisements

Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out / Change )

Twitter picture

You are commenting using your Twitter account. Log Out / Change )

Facebook photo

You are commenting using your Facebook account. Log Out / Change )

Google+ photo

You are commenting using your Google+ account. Log Out / Change )

Connecting to %s