Hello guys, this is the last and third article of our Code Review Series. I hope the Code Review Series is being helpful to all of you. In the last two articles, we have discussed the best practices from the perspective of a code reviewer. Today, we are going to look at the issues from the other side. Any guesses? The other side refers to the perspective of the developer. So let us discuss some of the best practices that a developer or code author should follow.
You might be thinking that the job of a developer would just be as simple as writing down the code, creating a code review request, and then asking the code reviewer to initiate the review. Honestly, I also had the same idea a couple of years back when I began my career in Amazon. Later on, I became the senior member of my team and started reviewing more and more codes from my colleagues. This led me to see things from the other side which made me realize that there are actually a lot of things which a developer could do to improve the code review efficiency and to make the most out of a code review.
It’s like if you have been an interviewer in your company for a while, you would get more understanding about how the interview process is and this would help you in becoming a better candidate during your next interview. Enough with the chatter, let’s start reviewing some of the best practises which a developer should follow during a code review.
The code reviewer should never assume that his/her own responsibility is to find code errors. The code reviewer mainly has to review the quality of the code in terms of readability, maintainability, and the logic of the program implemented as per the requirements. The bugs in the code should be guaranteed with the help of sufficient unit testing, functional testing, and integration testing. By sufficient, I didn’t mean the quality of the test cases but the actual test coverage of the test cases. For example, is each function in the program called? Is every branch of each control structure such as if and case statements been executed?
There are dedicated code coverage tools to do this such as JaCoCo library for Java. These test coverage reports should be then submitted along with the code review request.
Being a developer as well as a reviewer, I would like to say that examining your own code is quite important. Reviewing your own code is similar to self-introspection. You need to understand the purpose of code completely and then review your code. Once you are clear about the purpose of code,
start examining the code diff. You should yourself find out the basic mistakes from the code. Minor errors make a huge difference. The developer should ask himself/herself questions like: Did I leave a comment or TODO, Does that variable name makes sense? If you have gone through the first and second articles of Code Review series, you must recollect that I have included points like code readability and code efficiency. So do consider these points as well.
Please remember that the reviewer is qualified and not there to find out basic errors like syntax and format. Try to solve them yourself. Being a developer, you should definitely pass your own code review before submitting it for reviewing. On a lighter note, understand that it always stings less to get notes from yourself than from others.
A developer should always be open-minded. Always remember this because it will help you a lot. This is not a minor issue. If you are not open-minded and not ready for accepting changes, then it will always create trouble. Many a time, developers become defensive during code reviews when the reviewer points out a mistake or gives a better suggestion. This is my own experience! Guys, “Please do not take anything personally” when it comes to code review. Remember that the culprit of this story isn’t you, it is the defects or errors in the code. It is quite natural to be emotionally attached your code. In fact, taking pride in your code is the sign of a good developer. It means that you care about what you do! But always have the right amount of ego, having more than that would certainly hinder the progress of you as well as the project.
The reviewer knows that errors are meant to happen because we are humans and we aren’t perfect! Consider yourself and the reviewer on the same team. The reviewer just points out things that the developer might have overlooked. Questions are always valuable and never forget to ask specific questions like “Will it make more sense if I extract those three methods to a separate class”. Working together like this with a positive approach would truly help in achieving success. Do thank the reviewer for sharing knowledge, giving feedback, and spending time.
The frequency of your code reviews as a developer should be high. In all these years as a developer and as a reviewer, I have gone through many painful code reviews. This is because these code reviews were carried out at the end of the project. One should never forget that each code diff includes a million lines of code. Not just this, there are several features implemented and are also interlinked. Reviewing such a code is a headache for the first time. Just walking through the code consumes more than 3 hours! Sometimes, it becomes a compulsion to review the code and have to do it anyhow. The story doesn’t end here, most of the time, the reviewer’s thoughts and the developer’s thoughts won’t be matching. So this would lead to hundreds of comments and controversies. So reviewing the code is like building a house again even if the builder has almost built it!
So a good developer should always keep checking his/her own code frequently. This would help to create a firm foundation. The developer will then have fewer chances of receiving code rejection from the base. In fact, I would recommend that the developer should create his/her own checkpoints after which he/she should consult the reviewer. For example, in my team, everyone has developed the habit of reviewing their design ideas, then reviewing their backbone code and finally reviewing the complete code.
If the code seems to be complicated, then one must split the diff into smaller parts as per the functionality. A review should not include more than 1000 lines of code. If it is more, it would get boring and confusing.
Trust me on this guys, sending the code to different people is very important. I would especially like to convey that don’t just send the code to people who are your good friends. Remember that critics are the best friends because they would find out mistakes which you could have never found! For example, one of my friends used to forward his code to his friends. In return, he always used to receive friendly encouraging comments with minimum negative comments. The moment he started sending his code to other people, he received more negative comments. But this helped him grow.Please understand that we all have different ways of thinking and our opinions differ. Reviewers are not an exception, they will comment as per their own understanding. As per my experience, I have received comments on my code from numerous perspectives such as business logic, algorithmic efficiency, and scalability. So please be ready for it.
I have also tried anonymous code reviews. It is similar to anonymous feedback! The reviewer doesn’t know anything about the author and vice versa. Anonymous code review has proved quite effective. But always ensure that the code is being sent to a trustworthy reviewer. Even if the reviewer is a strong critic, he/she should be trustworthy. Generally, sharing your code with three people would be enough in terms of code review. The advantages of sharing your code in such a way are that the code will be reviewed from a different perspective, good in terms of knowledge sharing, and would also help in building a stronger relationship with the reviewer.
This article is different because the previous two articles of the Code Review series were from the Reviewers perspective. But this article is based on the developers perspective. In this article, we have discussed the best practices that a developer should implement before passing the code to the reviewer. It includes five main things. The first point is that the developer should not give up on code ownership and simply rely on the reviewer to find mistakes. The second or next point is that the developer should review his/her own article. Next, the developer should not take code review personally. After that, we have discussed the frequency of code reviews and then discussed the importance of forwarding your code to different people. For me, this journey of Code Review series has been awesome! I learned a lot of things by sharing my knowledge with you guys. I congratulate all of you for successfully completing this series and would definitely see you guys soon in the next series! Till then, keep improving, keep implementing, and keep enjoying!
Here is the link to the second article of the Code Review Series