DEV Community

loading...

Code review - from a PM's perspective

Hey, Dorka!
designer, team leader. love people, open to new stuff, currently interested in coding. <3
Updated on ・8 min read

Introduction

Introducing code review to an agile team requires patience, but 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.

Purpose of 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 a 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 quality

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 getting familiar with the coding guidelines. On the long run, this will lead to a general and commonly shared knowledge.
From project managers' point of view, it means that you might have confidence in a successful project. Such issues as sick leave or travels wouldn’t have strong impact on the outcome of the teams.

Knowledge sharing helps in increasing quality level. 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 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 on 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 the 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 to forget something next time. This positive culture will have 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 making review easier. (These are complement 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 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 around 45-60 minutes in a row. One review session shall not exceed this time interval. Few minutes break, a glass of water, fresh air, 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 repetition 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 to take notes, as 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 track all the bugs found during 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 the low level bugs our. It's an automated process done by 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 help in reviewing. Last but not least, the learning curve of newcomers will become better if they can access to high quality coding guideline.

Metrics

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

These metrics is because they point out potential code vulnerabilities, development process weaknesses. These can be helpful e.g. during root cause analysis). Performance can also be improved based on the 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 lines 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
  • Number of risk / Lines of code.
  • Number of risk / Function point.
  • 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

  • Inspection rateÉ provides a rough estimation on 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 inspection rate increases, defect detection rate decreases.
  • Defect correction rate: provides rough estimation on the time necessary for correcting known defects. This rate is used during planning of new development iteration.
  • Re-inspection detect rate: the amount of time spent with 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 in 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

General, most popular tool

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

  • To contribute in 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 amount 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 check statuses periodically.

Decision matrix

The management might want to measure and increase 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

Discussion (0)