Code review - from a PM's perspective

Written By Dora Makszy

10-Jun-2020

Introduction

Introducing code review to an agile team requires patience, but is remunerative. The team members will realise that they are actually benefiting from the process by…:

  • Learning the code base and new technologies.

  • Being able to perform estimation better.

  • Being inspired by sharing knowledge.

  • Facilitating daily routine by decentralising work.

  • Gaining new skills.

  • Having better interpersonal relationships by mentoring each other.

  • Increasing the quality of their own code, so they can be proud of it.

Besides, the industry standards need this kind of quality assurance method. The company is working on a complex system that is actually influencing lives.

The purpose of the code review

The aim is to have a maintainable, clean and understandable code. It is a realistic aim by…:

  • Finding and correcting errors

  • Avoiding duplicates

  • Refactoring and decreasing the complexity level based on an agreed coding guideline.

It is very important to highlight that code review is a team effort. It is a discussion, dispute or argument. The reviewer acts as a coach: (s)he asks questions and the author of the code tries to answer. While doing this, (s)he thinks the topic through again and solves the problem him/herself. This way it is not a passive process, but both parties are actively working and thinking together. Both can surprise the other by highlighting new perspectives.

Part of the code review can also be automated. It's recommended to avoid low-level, basic mistakes, spelling & grammar issues. Automation provides a framework where we can set guidelines to fit industry standards.

Learn and increase the quality

The passive approach is that the developer sets JIRA status as “In review”. So the ticket is assigned to someone else to read through the modifications and either accept or reject it.

Optimal would be to have these two developers sit together, going through the modifications line by line. They would ask questions to learn easier. They would search for alternatives. They would understand certain solutions and get familiar with the coding guidelines. In the long run, this will lead to general and commonly shared knowledge.
From the project manager's point of view, it means that you might have confidence in a successful project. Such issues as sick leave or travel wouldn’t have a strong impact on the outcome of the teams.

Knowledge sharing helps in increasing quality levels. Developers learn from their mistakes. They remember these sessions. They recognise (and avoid) the possible mistakes next time. The fact that their code is reviewed is a trigger to write it very carefully.
This mindfulness is definitely a very inspiring and intensive process. This results in smooth and fast code reviews and an increasing level of code quality.

A few thoughts on estimation

Developers are unique. Although they have common knowledge, they also bring something different to the team. By sharing knowledge, these different knowledge islands are getting closer, even overlapping. It has several advantages.

  • For example, a new and complex issue comes up at a regular grooming event. Developers can discuss the details, sharing their own perspectives. They can even highlight unforeseen possibilities or blockers. This way the project manager has the opportunity to eliminate risks.

  • Another example is when the senior developer mentors the junior. As they estimate tasks together, the junior will have a proper aim to reach. The junior developer will be motivated to do his/her job better. This will increase team quality. Again, the project manager will gain lower risks and higher confidence.

Best practices

Positive development culture

Each found mistake is an opportunity to learn something. About precision, new approach, or even not forgetting something next time. This positive culture will have a positive impact both on code quality and team attitude. It will lead towards generally demanding work morale.

Follow the 10 safety-critical coding rules

In 2006, NASA engineer Gerard J. Holzmann published ‘The Power of 10 Rules’. His intention was to improve coding quality, and make review easier. (These are complements to the MISRA C guidelines.)

  • Avoid complex flow constructs, such as goto and recursion.

  • All loops must have fixed bounds. This prevents runaway code.

  • Avoid heap memory allocation.

  • Restrict functions to a single printed page.

  • Use a minimum of two runtime assertions per function.

  • Restrict the scope of data to the smallest possible.

  • Check the return value of all non-void functions, or cast to void to indicate the return value is useless.

  • Use the preprocessor sparingly.

  • Limit pointer use to a single dereference, and do not use function pointers.

  • Compile with all possible warnings active. All warnings should then be addressed before the release of the software.

  • The last one is very hard to fulfil, but it sure does worth a try.

Consider human limitations

Humans have finite capacity and their brain and body must regenerate. There has to be a maximum level that can be reviewed at a time. Several studies showed that the average amount of code one can review is around 400-450 lines. Time is also a key factor in code review. Humans can focus for around 45-60 minutes in a row. One review session shall not exceed this time interval. A few minutes’ break, a glass of water, fresh air, and eating some fruits or dark chocolate will definitely refresh the mind and the body.

Reminders and checklists

There is a proven fact that more or less 5-6 repetitions will result in deepened knowledge. That’s why if the developer realises that (s)he did the same mistake for the second time. (S)he should consider taking notes, as a kind of reminder. Recurring mistakes and bad old habits might be avoided by using checklists.

Annotations and comments

Once the developer is reading through the modified code, (s)he might find more mistakes. These can be corrected on the fly. (This will result in lower defect density later.) To help the review process, it’s practical to add comments. These comments and annotations are guiding the reviewer through the code easily. These will help to prepare high-quality documentation.

Process, traceability and automation

Automated code review tools help in precisely tracking all the bugs found during the review process. These tools are providing the opportunity to generate regular reports.
It's very crucial for a project manager to be able to track the ratio of new features vs. found bugs. This is a factor of how (s)he calculates the budget, time and resources. Also, defect density, inspection rate, coverage of reviewed lines and code quality can be followed easily. Continuous integration can be fostered.

In addition to the above, such reports can support external audit processes as well.

Static analysis & coding guideline

Static analysis tools can help in filtering low-level bugs. It's an automated process done by a specific tool, triggered automatically.

Coding guideline needs to be created and maintained. Anyone who needs some hints on naming conventions, code structure, practices and methods, will find it quickly. Maintaining the coding guideline helps in reviewing. Last but not least, the learning curve of newcomers will become better if they can access to high-quality coding guidelines.

Metrics

Absolute metrics are objective, numerical values, such as the lines of code. Relative metrics represent subjective and non-measurable attributes.

These metrics are because they point out potential code vulnerabilities and development process weaknesses. These can be helpful e.g. during root cause analysis). Performance can also be improved based on absolute metrics, as well as efficiency and accuracy.

Secure development metrics

  • Lines of code: number of executable lines (comments or space do not count) is a rough estimate to quantify the size of the code.

  • Function point: estimation of code size based on measuring functionality.

  • Defect density: average of programming faults per line of code helps in to have a high-level overview of the general quality.

  • Risk density: average of programming faults, categorized by risk. Possible calculations might be

  • The number of risks / Lines of code.

  • The number of risk / Function points.

  • Cyclomatic complexity: complex code is less stable and hardly maintainable. Cyclomatic complexity helps in establishing risk and stability estimations. It provides the following intervals:

  • 0-10: stable code, low complexity.

  • 11-15: medium risk, acceptable but higher level complexity.

  • 16-20: high risk and complexity level, too many decisions required.

Review process metrics

  • The inspection rateÉ provides a rough estimation of the predictable duration of the code review process. It gives the amount of code reviewed in a specific time interval. (Average pace is 250 lines of code reviewed in 60 minutes.)

  • Defect detection rate: number of defects found in a specific time interval. As the inspection rate increases, the defect detection rate decreases.

  • Defect correction rate: provides a rough estimation of the time necessary for correcting known defects. This rate is used during the planning of new development iterations.

  • Re-inspection detect rate: the amount of time spent reviewing the same code, plus those lines that are for correcting the defects found earlier.

  • Code coverage: proportion of code reviewed (percentage of lines of code per function point). It is practical to build safety checks, so the build will fail if the coverage is under the desired percentage. This is to ensure quality and avoid logic errors.

Tools

The general and the most popular tool

Software development companies usually use GitLab in-built solutions. It is connected to Atlassian JIRA. This tool enables the developers…:

  • To contribute to development by discussing new approaches. To provide their ideas to further disputation without causing irreversible changes.

  • To have the ability to compare original lines of code with newly added ones or specific modifications.

  • To check for related references in already accepted commits.

  • To check the number of changes over time.

  • Easily solve conflicts by using merge pull requests.

  • To control code quality by giving specific permissions to collaborators. It also protects branches and checks statuses periodically.

Decision matrix

The management might want to measure and increase the effectiveness of the code review process. It is straightforward to set up a decision matrix. It helps the team can decide how to get metrics in an easy, controlled an automated way.

References

Articles

Tools

Additional reference

Dora Makszy https://www.heydorka.rocks/
Previous

Why UX is your best friend?
Next

A different angle