In this post, I will try to answer 2 questions. What is code review? Why we are reviewing the code? What is Code Review? Code Review is the activity that one or more software engineers (except the author) is reading and checking the code. Who is checking the code is called as reviewer. At the end of this activity, reviewers give their feedbacks for the code. Description seems very simple but if you look deep into code review process, you will see that it is more complicated. To get a better understanding for the code review process, I will try to answer “why we are reviewing the code” question. Why we are reviewing the code? Answer of this question varies according to the industry, team structure, product which is worked on. At the below, I will try to give an answer to the question “why we are reviewing the code” for the most common cases 1. Check if code is working right Generally all reviewers are checking first if code is working right or not. Since code review is a time-boxed activity, reviewers are checking common error prone code firstly to make an efficient code review in given timebox. Here are the common error prone code pieces 1.1 Loops Check if loop condition is satistified ever Check if loop exit condition is satisfied ever Check if iteration number is the expected iteration number — Off by one error is a possible error for the loop iterations. You design the loop iterate n times but it iterates n-1 times (for more details: https://en.wikipedia.org/wiki/Off-by-one_error) 1.2 Conditionals (If/else) and Comparison(<,>,≤,≥) operators Conditional operators are the branching points for code execution. Based on the evaluation of logical statements, code is executed from different branch. Logical statements of the conditional operators are open to errors. In case of an error in conditional statements, code executes in a different way then it is designed. I will give a link about the common misconceptions and errors for the conditional operator. Reviewer should be checking possible misconceptions and formatting errors. : Misconceptions https://stackoverflow.com/questions/47959876/which-are-the-most-common-pitfalls-with-conditional-if-statements-in-c/47959877#47959877 : Errors https://stackoverflow.com/questions/47959876/which-are-the-most-common-pitfalls-with-conditional-if-statements-in-c/47959897#47959897 Additionally it is also a possible to create a logic error in conditional operators. Specially for the boundary conditions, reviewer should be more careful. Possible errors using < instead of ≤ and vice versausing > instead of ≥ and vice versausing && instead of || and vice versa 1.3 Null Checks “Null reference exception” is one of the most common mistake for software developers. Code Review is an important opportunity to decrease the frequency of this mistake. There are also code analysis tools which warns us for the possible null reference exception.( ) Resharper null reference documentation Bonus: C# has nullable types that allow you to assign null to value type variables. It can be a need for some use cases like Database keeps true, false and null as boolean valuesThere is a nullable database field For details of nullable types: https://docs.microsoft.com/en-us/dotnet/csharp/language-reference/builtin-types/nullable-value-types 2. Check if the code satisfies the requirement It is obvious that, testing is the main activity to check if product satisfies the given requirements or not. We need to consider that, how late the error is found it costs higher. Based on this fact, I suggest to execute a basic test. We have already discussed for basic tests in the team and we have decided to apply it according to our past experience. Requirement documentation, use case documentation and code commit comments can be helpers for reviewer to check if code is satisfying the requirement or not. Tip: Code commit comments are real helpers for review process. Code commit comment must answer the questions given below What you have done in the code?Is there any use case or requirement related with your implementation?Is there any RQ number related with your implementation? 3. Check if coding guideline/coding conventions are violated Every software developer has a development style and tries to implement code with his/her own way. It should be limited to avoid mess in the code base. Coding guidelines and conventions are trying to avoid this mess. If there is a usable guideline document or conventions, reviewers are checking the code if there is a violation. 4. Collective Code Ownership and Code Reuse Maybe you heard the term “Collective Code Ownership”. It is defined in Extreme Programming. It is also called as “no code ownership” by Martin Fowler. (https://martinfowler.com/bliki/CodeOwnership.html) In simple terms, it means that any member of the software development team can make any changes on anywhere. It is always a difficult job for the teams because every member needs to know about every part of the source code. Code Review activity is a great opportunity to create the basis for “Collective Code Ownership”. I am sure that you have implemented a function which is already existing in the codebase. It is a common use case for the software developers working on big codebases. If you can use code reviews in an efficient way, you will have a better chance to reuse already existing code blocks. 5. Self Improvement Most of the software developers are in love with their code. If there is any negative feedback at the end of the code review activity, it can be misunderstood and personalized. I have a suggestion to break this negative atmosphere for code review activity. Reviewers must try to benefit from the rewieved code as much as possible. I have seen that developers do not give any positive feedbacks to each other’s code. If you try to utilize the code and try to learn from the source code you are reviewing, you can also give positive feedbacks for the good code. I have experienced that, positive feedbacks are creating a much better atmosphere for the code review activity. It is a common belief among the developers, they think that developers can improve themselves only by writing new code. It is obvious that, learn by doing is a useful concept. But we need to consider that reviewing other developer’s code is another way of the improvement. In my experience, I have learnt some of the new features of the programming languages and more importantly another way of thinking for problem solving during my review activities. 6. Code Readability If a developer gets a legacy code, most probably the first complaining point is code readability. Even I read my source code 6 months later, I always ask who wrote this messy code:) Developers always sacrificing readability for other cool features and they feel themselves better. I think that there is no trade-off between readability vs others (Even there are opposing view for my thought) readability vs performance readability vs simplicity readability vs .. For these reasons, code readability should be one of the main reason why we are reviewing the code. Even the variable names and function names can have a big impact on code readability. There is also a famous quote by Martin Fowler. “ Martin Fowler, 2008.” Any fool can write code that a computer can understand. Good programmers write code that humans can understand. Review is your last chance the help your colleagues to write a code that human can understand. Final In this post, I have tried to describe what is code review and try to identify the reason why we are doing this activity. At the next post, I will try to answer “how” question. I will give details about how we are reviewing the code. Special thanks to and for their review and valuable suggestions. Gokhan Topcu Halil Kiracı Thank you for reading.
