Code Reviews

Code Reviews

We use code reviews as a mechanism of checking code against set standards and also to check for logic bombs.  A logic bomb is an error in the logic of the program that might not manifest itself immediately:  it may only arise in a year’s time.

Why Code Reviews?

Our aim is twofold, namely to encourage standards-based coding and to check for possible errors.  At the end of the day we want to minimize the maintenance load on our programmers and provide our clients with a quality, and stable, product.

Obstacles to Code Reviews

When we implemented the concept of reviews we had to do it carefully.  Programmers do not like being told how to code.  We tackled that issue very early on:  we did not want to prescribe to them how to think and code, rather what standards to follow in creating certain objects (variables, hints, tooltips, X and Y positions of the first field on a screen).

Another obstacle is “Who pays for this?”.  We have a team that codes customized software for clients and they work on an hourly basis.  When we do a review and find something wrong we are always asked who pays for the fix.  This battle is still ongoing.

How do we schedule the reviews?

Each Monday morning I draw a list of jobs that are still open and have a look at the programs linked to them.  To choose which code to review I consider the programmer, program and importance of the job.  I know that certain programmers might struggle with certain technologies, hence they get reviewed more often.  In our case this actually led to the programmer writing better code, as the standards were always in the back of her head.

Certain programs are marked as sensitive.  This means that when you touch it, a review will be requested.  Sometimes the review is done by myself, other times by a system specialist.  A senior developer will check both standards and logic as they know their system better than most.

For jobs that are sensitive or high priority I will generally try and fit them in as well.

Once the list has been compiled I block out 30 minute slots for each review on my calendar and send the line managers the list to approve and schedule their people.

Recording of reviews

Each review is recorded on our internal ERP system with statistics being drawn monthly.  These stats give us an idea of who is struggling with a certain technology and may require training, or which programs occur more often.  Management then makes decisions on how to progress from there, for example a program might be placed on the sensitive list to be reviewed on each change.

Programming Standards

Where are the standards defined?

We have a programming standards manual that is owned by the head of Service Excellence.  When new standards are developed or old ones updated, this manual is changed and published on the quality management system.

How are new programmers introduced to the standards?

During induction we take new programmers through the standards.

What we’ve learnt over the years is that junior programmers should not be taken through the whole manual:  you lose them within 15 minutes!    To this end we’ve created summary pages for our different tech standards.  For instance, the standard on ‘C’ programming is about 20 pages long, but the summary is 2 pages long, in table format.  We’ll work through the summary and allow them to refer to the detail chapter for more information.

Senior programmers actually love the manual and we discuss each chapter in detail.  We’ve found that new senior programmers appreciate standards being set.  It introduces them quickly to the code and allows them to get productive much quicker.

Do you do code reviews?  Are they useful?  Let me know.


Leave a Reply

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

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

Google+ photo

You are commenting using your Google+ 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 )


Connecting to %s