The Art of Reviewing a Code

5 Cloud Native Trends to Follow in 2020
June 20, 2020
CODE REVIEW SERIES
Reviewing a large chunk of code changes
August 11, 2020
Show all

The Art of Reviewing a Code

CODE REVIEW SERIES

Usually, when new developers are asked to review codes, they often end up in a confused state. Some of our new members have already asked me questions regarding it. So based on my experience of all these years, I have come up with some guidance on “What all should be done in a code review?” So allow me to share it with you today!

What do you mean by code review?

I assume that most of you must be aware about the basics of code review or what code review means. For all those who don’t, let me explain it in a simple way. Code review is nothing but the inspection of code that has been written down by someone else. One has to first go through the code thoroughly and then find out defects or loopholes. In a positive sense, one has to seek for further improvements in the existing or provided code.

How does Google deal with code review?

I have been working with Google for a couple of years now. Google is as cool from inside as it seems from outside. The company accepts changes and improves. As a software engineer, I could simply tell you that Code Review has always been the backbone of every project that includes programming.

Not only is the code reviewed by our seniors, but it is also reviewed by peers. 

There are a couple things inside Google that aren’t confidential. They have been discussed globally and a lot of companies have benefited from it. That is what I want to discuss with you. Google’s code would always seem simple and this is because of Code Review. No other company practices Code Review on such a large scale. My team at Google has a special rule that no code for any project or product gets checked until it receives a positive review. 

Trust me guys, this is an awesome practice and shouldn’t just be informal. Every company should practice such rules on a larger scale if they are serious about software development. Code Review techniques are easy to implement and aren’t stressful. Google has benefited a lot from it and has made a huge difference.

Through the eyes of a Code Reviewer

Finding mistakes or errors isn’t easy. Especially, when the project is on a large scale it gets even more difficult. So it is very important to know the basics of code review. So now we are going to discuss all the major points which every reviewer should consider. 

The first and most important thing is to figure out “The purpose of code” 

“Why?” is one of the most important question. It leads to the necessity or purpose of doing anything and takes you to the root level. For example, you are reviewing some files of any project. Before doing that, figure out the purpose of the code change. Now you start reading the feature description and other information related to the files. While you are going through it, you note the problems/errors/suggestions/changes and make a list of it according to your thinking. Once done, think “Why” the author has made particular changes. Are those changes intentional or accidental? What was the author’s possible approach towards the code? So this comparison and questioning is quite important.

The second important thing is to understand The Architecture/Design of the code change

  • The beginning of knowing The Architecture/Design should be done using The Single Responsibility Principle: While code reviewing, the Single Responsibility Principle should be followed. According to this principle, a “class” should only have one responsibility. Now, this certainly sounds weird but it is true. This also stands true for “methods”. While you are talking about the level of abstraction, one should never use “and” to finish describing a “method”. For example, Method X has arguments {searchUsersAndAggregateResults}. This can be broken into two methods {searchUsers} and {aggregateResults}. 
  • The second thing is to determine Code Duplication: Duplication of code is literally a threat. For getting rid of it, one needs to practice The Three Strikes rule. According to this rule, a code can be copied once but if it is copied more than once, it should be extracted into a new procedure. This will help in splitting out the common functionality. Code duplication always makes it difficult maintain the code. So this is important as well.
  • The third thing is to take care regarding Code Efficiency: Efficiency is something which can always be improved when it comes to coding. Most of the codes have algorithms and this is where usually efficiency is tested. Here, the reviewer should ask the question to himself/herself, “Is this algorithm giving an optimum solution?”  For example, iterating a list of keys in a dictionary for locating the desired value will never lead to an optimum solution. For this, a hash map might be an optimum solution!

The next thing after Architecture/Design on which a code reviewer highly emphasizes is Code Readability

Generally, developers don’t keep writing code but they actually spend more time of adding, removing, and editing small pieces of code. While the code goes through all this, it should still be readable. A readable code is like a readable text. Developers hate reading or going through the sandwiched code. Places where nesting is required, should be written or sorted out in such a way that minimum amount of conditions are implied. For example, “If-Else” conditions can be written in numerous ways, but the one which gives the same output with a minimum number of curly braces {} and conditions is the optimum code. If the length of the method is long, the method itself should be broken. For example, sometimes i come across method’s having 500 lines. It certainly gives me a headache.  Overall, it is not about the length of the code, but it is all about the neatness and readability of the code. A good readable code will help developers troubleshoot errors easily and happily.

After Code Readability is ensured, the fourth important thing is Testing or Test Cases where the code undergoes rigorous testing

Test cases are certainly important. According to me, until you don’t put your code under test cases and difficult situations, you won’t be able to know how capable it is for the market! These are the few points that I believe should certainly be considered when it comes to testing.

The first thing regarding Testing is to decide the Test coverage: Which are the modules that   require testing? Almost all the modules are important but I would specifically emphasize on the ones which include new features. Are they thoughtful and do they include all the failure conditions? Are they readable and how about its fragility? Are the tests big enough and is the output slow? Such questions are a part of test coverage. 

The second thing regarding Testing is to ensure The level of testing: One must ensure that the testing is at the right level.  While reviewing tests, I always make sure that we are at the required level to check the expected functionality. Gary Bernhardt recommends a ratio of 95% unit tests, 5% integration tests. As per my experience, I have also found this approach quite effective with Django projects. It is easy to test at a high level by accident and create a slow test suite. This explains the importance of being vigilant.

The third important thing about Testing is judging the Mocks: Mocking always leads to good results and mocking everything isn’t good. There is an unofficial thumb rule which mentions that if there are more than 3 mocks in a test, it should be revisited. This only means that either the testing is quite broad or the function is large. Sometimes, a simple integration test could also be sufficient.

The fourth thing regarding Testing is to find out whether everything is Meeting expectations or requirements: While the review is on completion, I think that the user should have a look at the basic requirements of the story or the bug. If it is not matching the criteria, then one should bounce back and find the loopholes. I have been following this method

Conclusion

In this article, I have talked about the four most important aspects of Code Review. Beginning with the basics like “what is code review”, you now at least have a hint of “how my team at Google does code review”. I further explain the basic flow of code reviewing. It includes four main sections: The Purpose of Code, Architecture/Design, Code Readability, and Testing or Test Cases. These sections further have sub-sections respectively. So with all such information and practices, I certainly believe that it will help you become a good code reviewer. So keep practicing, keep reviewing, and keep improving!

James Lee
James Lee
James Lee is a passionate software wizard working at one of the top Silicon Valley-based startups specializing in big data analysis. In the past, he has worked on big companies such as Google and Amazon In his day job, he works with big data technologies such as Cassandra and ElasticSearch, and he is an absolute Docker technology geek and IntelliJ IDEA lover with strong focus on efficiency and simplicity.

Leave a Reply

Your email address will not be published.

LEARN HOW TO GET STARTED WITH DEVOPS

get free access to this free guide, downloaded over 200,00 times !

You have Successfully Subscribed!

Level Up Big Data Pdf Book

LEARN HOW TO GET STARTED WITH BIG DATA

get free access to this free guide, downloaded over 200,00 times !

You have Successfully Subscribed!

Jenkins Level Up

Get started with Jenkins!!!

get free access to this free guide, downloaded over 200,00 times !

You have Successfully Subscribed!