visit
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 pieces1.1 Loops
Check if loop condition is satistified everCheck if loop exit condition is satisfied everCheck 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: //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:
Errors:
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 errorsusing < instead of ≤ and vice versausing > instead of ≥ and vice versausing && instead of || and vice versa1.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.( )Bonus:C# has nullable types that allow you to assign null to value type variables. It can be a need for some use cases likeDatabase keeps true, false and null as boolean valuesThere is a nullable database fieldFor details of nullable 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 belowWhat 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. (//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)There is also a famous quote by Martin Fowler. “ Any fool can write code that a computer can understand. Good programmers write code that humans can understand. Martin Fowler, 2008.”
Review is your last chance the help your colleagues to write a code that human can understand.