eZ Community » Blogs » Arne Bakkebo » Code review considerations - QA#5

By

Arne Bakkebo

Code review considerations - QA#5

Tuesday 21 May 2013 9:58:35 am

  • Currently 5 out of 5 Stars.
  • 1
  • 2
  • 3
  • 4
  • 5

In the previous article I discussed code repository branching and code review procedures. Today I will follow up this by talking about what to focus on when doing the actual review of a code change.
[QA4 - Branching and code review procedures]

I have made a list with a few points to keep in mind when doing a code review:

  • Discuss principal questions
  • Consider naming conventions
  • Consider coding best practice
  • Think about intelligible code commenting
  • Think about clean and readable code
  • Clean up old related code

Notice that all of these will relate heavily to the best practices post I made earlier, and may also partly overlap with each other. I will talk a little about each of these elements in the following.

Discuss principal questions

When doing the review, consider principal questions relating to best practices. What is the best way to solve a particular coding problem? Don't be afraid to bring up alternative solutions, and have a dialog on which solution may solve the problem in a good fasion.  Having a dialog is what code reviews are all about.

Consider naming conventions

Notice the names that are used for functions and variables, consider what they tell you, and consider what they might tell anyone who doesn't know the code well. Names used are a big component in the documentation of the code. The better names you have, the less you are required to comment the code and still have it easily understandable.

Consider coding best practice

Review the article I recently wrote on development best practices, and consider the different elements mentioned there in the review.

Think about intelligible code commenting

Consider how the modified code is commented. Are the functions input and output clearly defined and explained? Have the purpose of the functions been explained clearly? Are there complicated code parts that could be clearified with an explanation?

Consider also if any existing comments are superfluous and can be removed. Do they explain the code, or just duplicate it? Avoid comments that does not clearify anything to the reader, it will only make the code longer and make the job of maintaining the code harder.

Think about clean and readable code

Again, consider best practices. Will someone that does not know the code be able to easily see the purpose of the code? Are there parts that may require time to understand? Is it possible to refactor these parts to make them more easily understandable? Do not avoid taking the time to refactor bad code. This is what will improve maintainability of the code in the long run.

Clean up old related code

Endevour to improve the whole code base of the project over time. If you do not, the entropy of the code base will increase, and the whole project will get harder to work with. And that ultimately means there will be more bugs that you need to fix, taking time from creating new and better features. If you consider in the review how each section of code can be improved, the project will slowly improve as a whole.

Conclusion

The main conclusion you should draw from all of these points is to spend some time on each code review. Try to understand the purpose of the code you are reviewing, and consider alternative solutions to achieve the same purpose. Don't be afraid to start a discussion on best solution for a given task, this is usually educational for everyone involved. I'll repeat myself by pointing out that catching problems in the code at an early stage of the development process will save you a lot of time in the whole project[1], so spending time on the code review is imperative both for avoiding problems and improving readability while saving project time. And keep in mind that doing a code review well requires practice.

Finally, one thing that many people think they should look at when doing a code review is code formatting. However, I recommend not starting a discussion on this in the review, since it removes focus from the real purpose of a review and ends up being a waste of time both for the code owner and the reviewers. The code formatting should be decided on and done properly before the code is sent to a review. If it is not, send it back politely and redo the process when the work is properly done.

I imagine there are aspects of a review I haven't thought about. Feel free to add your own considerations to what it should include, or tell me if you disagree on any of these.

[1] Speedy software development: http://www.stevemcconnell.com/articles/art04.htm

Proudly Developed with from