Is Code Review as important as Salary Review?

Is Code Review as important as Salary Review?

Hi everyone, I'm Nahut from EST team. As the article title reveals, today I would like to share the topic "Is code review as important as salary review?". I hope everyone will contribute and discuss this article with constructive comments. Let's get started!
Is Code Review as important as Salary Review?

My story

I still remember vividly the first days of my internship; I created a PR (Pull Request) to ask my team members to review and received dozens of comments. I dragged the mouse until I got to the last comment. I also wonder, "Why do people comment so much?". That's when I realized I had an excellent opportunity to learn from them.

Some abbreviations

1. UT = Unit Test
2. PR = Pull Request
3. CL = Change Line​

What? What is code review?

According to Wikipedia:

Code review is a software quality assurance activity in which one or several people check a program mainly by viewing and reading parts of its source code, and they do so after implementation or as an interruption of implementation. At least one of the persons must not be the code's author.

Code review is the process where programmers review and evaluate the code of another member of the team/project to ensure this feature was implemented correctly and adequately.


Why should we review code?

  1. To ensure PR runs correctly, meets customer requirements, and limits bugs due to missing specs.
  2. Discussing constructively among team members will help improve the project's code quality and reduce technical debt.
  3. Additionally, having a code review checklist will help members comply with the team's flow and code convention and the project or company.
  4. Code review activities are also a way for team members to share knowledge with each other, which is really helpful for new members to catch up on the project faster.

    6 Reasons Why Code Reviews Are Especially Important for Distributed Dev  Teams - DistantJob - Remote Recruitment Agency

Who? Who will be responsible for code review?

Every dev team member can review code for each other, regardless of role or experience.

Normally, the experienced developer or team lead in the project, who understands the system/business operations of the project best, will be the one to finally decide whether the PR is eligible to be merged or not.

What should be considered when you do a code review?


Code Is Never "Perfect", Code Is Only Ever "Good Enough"


We could focus on the following factors when reviewing code:
   1. Design
   2. Readability & Maintainability
   3. Functionality
   4. Tests
   5. Performance
   6. Security

   1. Design
     - Do CLs comply with the project's design patterns? (DRY, Service Object, Query Object, Serialize ..etc)
     - Are CLs compatible with the current architecture of the codebase?
     - Can CLs reuse old functions?
     - Are CLs affected by other functions or screens?
  
   2. Readability & Maintainability
     - Is the naming method (class, variable, field, parameter, methods) easy to understand and accurately describes its function?
     - When reading Unit Test contexts, do you understand the purpose of what the code is doing?
     - Are comments appropriately written?
     - Is the Docs API updated when new API additions/modifications exist?

   3. Functionality
     - Is PR working correctly and according to requirements?
     - Does PR work on other dev machines? (Ensure compatibility on different environments)

   4. Tests
     - Is there UT(Unit Test) for new/modified code?
     - Have the Unit Tests covered all the test cases? (Test Coverage C0, C1, C2 will depend on the project)
     - Have UTs missed any important cases?

   5. Performance
     - Do CLs affect current performance? Does it cause slow requests?
     - Missing debug command on code(binding or byebug for Ruby), which could cause time out to the server.
     - Issues that can affect performance such as:
       + Calls to other services time out
       + Handling with database:
         + N+1 queries
         + Deadlock
         + Slow queries
       + Issues related to threads, CPU, RAM..etc

   6. Security
     - Use tools to check basic security errors, if any (SQL Injection, Cross-site Scripting, etc.)
     - Has important data been encrypted?
     - Check to see if project keys are exposed.


What are the advantages and disadvantages of code review?


Advantage:
     1. For personals:
       - Improve your own skills
       - Catch up the project faster for new members
       - Sharing knowledge among team members
       - Learn how to communicate and give feedback to others, as well as verify your own knowledge.

      2. For projects:
         - Reduce missing requirements
         - Improve project code quality and Minimize bugs
         - Reduce technical debt


Disadvantage:
       1. Review code also takes time and resources, especially with large PRs or PRs with many feedback comments, having to review many times (This often happens to new members who are not familiar with the flow or have not understood the projects

       2. Can cause internal conflict in the team between members when there are many conflicting comments. This cannot be avoided. People should attach a reference link to make it more convincing.
         ​

Summary

Many developers are afraid of reviewing code because it takes time. I completely agree with this. Code review could take 30 minutes to 1 hour or longer if there is a lot of complex logic and you must fix comments many times. In general, code review will bring many benefits to project quality and improve individuals, which has more positive than negative things.

Project quality increases, customer satisfaction increases, revenue increases, and salary also increases (Just for fun) ^^

More like this

A Summary of Live Coding a Simple CLI for gRPC Testing at Go Conference 2023
Jul 14, 2023

A Summary of Live Coding a Simple CLI for gRPC Testing at Go Conference 2023

Bug Bash? How To Run It?
Jun 15, 2022

Bug Bash? How To Run It?