Need Help in Pointing to focus on the Key elements in Code Review Phase? [closed]

Question

Please share your views on the code review process. If someone gave a code snippet and asked you to review that code, then what are the major things you will focus on that code Review process?

For instance, I will check if any dead code is available in that code, other than checking dead code, what are the key elements to be focused on in the code review process?

Update: Please share any free code review e-book which helps me to understand more and more.

Answer 1

I use the following check list. Many of these are from "Clean Code" book and SOLID principles.

Logic

• Code duplication
• Code is in the wrong place. For ex. presentation logic is in business logic layer and vice versa
• Class has too many responsibilities. For ex Order - that handles payment, shipping, etc... Instead those responsibilities should be moved to other classes.
• There are appropriate unit tests that provide good code coverage

Readability

• Function is too long, tries to do too many things
• Function/class names are not clear
• Follows project's standards and naming conventions

Speed

• Is the code sufficiently optimized
• Can it affect adversely other parts of the system
• Would there be issues under load

Answer 2

Since you specifically ask for e-books in your update, I'll point out that my company, SmartBear, gives away a book on code review. Unfortunately, it looks like you're in India which is not one of the countries we currently ship to. However, you'll be happy to note that the signup page linked above, includes links to several sample chapters from the book.

Our general recommendation on building code review checklists, is to spend time analyzing the bugs that you write. You need to go deeper than the individual cases and think about the underlying causes or classes of bugs.

Beyond the empirical data, there are some things that are necessarily bugs, but should be verified in code review to ensure maintainability of the software:

• Are there sufficient unit tests?
• Are classes and methods sufficiently and clearly documented?
• Are classes and method appropriately scoped?

Finally, if your organization is just adopting code reviews, start slow and be patient.

Answer 3

Sounds like your organisation could benefit from keeping a code review checklist. Write one! They make reviews more effective.

• Start with suggestions from the other answers here, or from books like Code Complete.
• Add more items by looking through your bug database and finding any common coding mistakes that are responsible for many bugs.
• Encourage everyone to use the checklist in reviews.
• Keep the checklist up-to-date. From time to time, you will want to add new items and remove old ones.

Answer 4

(1) Does the code solve the problem you are trying to address? (2) Does the code clearly communicate the solution? (3) Can the code be optimised to the degree of (1) and (2)?

Obviously, your foucs may change if you are mentoring a junior/graduate developer. You may find he/she has bad programming habit, such as using magic numbers, duplicate code, excessive variables (the list goes on..) and you want to provide some feedback to help improve the developers coding skills, but I've have the above theree questions in mind and try to steer people towards that in my own reviews. You obviously want the feedback of your code review to be valuable and carry across to other developers.

Answer 5

Did you try looking at Fagan's inspections? You need not use the full blown inspection methodology/checklist but you'll surely benefit from the insights.

Answer 6

During code review, with our projects, I've always started out by noting entry points and from there following the data pipeline(s).

Find each stop and each flow change or data alteration along the way until you have a grasp of all the course(s) data will take from start to finish. Compare the code to the function the code is actually performing.

• Could the pipeline be simplified or does it need restructuring?
• At each stage is data coming, going and operating as expected and intelligently?
• Check the loops, check them again and again. Consider them often.

After becoming satisfied with the above, focus usually will shift to other factors.

• Can the code be simplified by abstraction/unabstraction/better syntax or naming?
• Review serialized data structures like arrays and vectors, sometimes variable should be promoted from these to their own identity and other times variables should be collected into them.
• Can the code be simplified? (Just look harder.)
• Second restructuring and restarting from the pipeline if changes are significant.