DEV Community

Cover image for How to become a better Code Reviewer - Part 2 : What does a Code Reviewer look for?
Divakar Kumar
Divakar Kumar

Posted on

How to become a better Code Reviewer - Part 2 : What does a Code Reviewer look for?

What Do Code Reviewers Look For?

  • Design:

So what do i mean by design? I refer to design by the overall design of the change list.

1) Check whether all the interactions of code in the change list make sense to you.
2) Check whether the change is from library or the actual codebase.
3) Check whether it integrates well with rest of the system.
4) And also make sure whether it is a good time to add this functionality.

  • Functionality:

1) Check whether the author of the code have done what he/she was intended to do based on the change list.
2) Think in terms of the end-users and also the developers who is going to use this code in future .

3) Think about the edge cases , looking for concurrency problems , and making sure that there are no bugs that you see just by looking at the code.
4) If you don't have any idea about what is going on , you can have the developer/author of the code to give you a demo of the functionality before going through the code.
5) If any kind of parallel programming is going on in the change list make sure there are no deadlocks or race conditions

  • Code Complexity:

1) If the author of the code approached more complex way and also in a way it can't be understood quickly then it means that he/she is likely to introduce bugs when they try to modify this code in future.
2) Generics are good but not always. A developer should know when it is likely to be approached using generics and when not to.

  • Unit Testing:

1) Never approve a pull request that doesn't have corresponding unit test, integration, or end-to-end test. In general , tests should be added in the same change list

2) Check whether tests actually fail when the code is broken , do they make simple and useful assertions , are they separated appropriately between test methods.
3) Tests are equal importance as of to the actual code base , so check for complexity in unit test as well.

  • Naming:

A valid name is enough to fully communicate what the actual functionality is.

1) Check whether the author of the code uses good/valid name for everything . It can be variable, method,class declaration literally everything.

  • Comments:

Comments are useful when they explain why some code exists and it should not be a place where to explain everything the code is doing. If the code isn't clear enough to explain itself , then it should be made simpler.

1) Check whether there are clear comments and also whether those comments are actually necessary.

  • Documentation:

1) Ask for documentation from the author of the code and also ask them to update associated documentation for the code change that they made
2) Also consider whether a part of the documentation need to be deleted or so, when the particular functionality is removed.


Next Steps:

Banner Image Courtesy : Business vector created by freepik -

Top comments (0)