samedi 12 octobre 2013

What is a good code review?

Every programmer does mistake in his code, whatever are his programming skills. You can detect these mistakes at different level in the development workflow : from the development itself to the production. What is sure is that the faster you will find a mistake, the faster you will correct it.

Reread the code, even if it works, is a good way to find potential bugs as fast as possible. This reading is called the code review.

Every team has his own way to do a code review. Some ways are bad, others are better. In this thread, I will present you what is for me the perfect code review, and we will see that a good code review has others benefits that just reveal potential bugs.

How?

The reader and the programmer must read the code together, side by side using the programmer computer. It encourages both of them to discuss and debate about the code in order to challenge it as most as possible.

The programmer must not introduce the modifications to the reader because the reader must understand himself the code. Indeed, the code must be clear enough because other teammates will have to work on it. So the reader must take the mouse and keyboard command.

To be sure to read every modified line of code, the review must be done from the modifications diff of the version control software. Of course, if necessary, the reader can have a look in the entire code.

If the reader see something which must be changed in the code, the programmer must take a note to don't forget to change it after the code review. Every modification must be done just after the code review.

What?

The reader has the responsibility to approve that the code is correct. If he doesn't see a bug during the code review, he will have the same responsibility than the programmer if a problem occurs in production. So every line of code must be checked and understood.

Several kind of problems can be detected in the code and fixed with the code review. For examples, the reader could do the following remarks to the programmer :

  • "The format of these two lines of code doesn't respect our coding conventions"
  • "When I read the method name, I don't understand what is his responsibility"
  • "In this case, you will have a NullPointerException with this variable"
  • "For this line of code, there already exists an utility method in the util package"
  • "A unit test is missing for this case"
  • "Here, you are doing a database access in a loop, you should refactor your code"
  • ...
As you see, the code review allows to be sure that the code will respect the coding conventions, that the code will be understandable, and that potential bugs can be detected.

When?

When the development of a feature is finished, the developer has to demonstrate his work and to ask somebody to read his code.

If the developer starts with the code review and finishes with the demonstration, it is possible that he has to modify his code after the demonstration because the feature doesn't fit all the functional requirements. So a second code review will be necessary.

On the other side, if he starts with the demonstration and finishes with the code review, it is possible that he has to modify his code. But in this case, as the developer directly understood the functional requirements and wrote regression tests, it is not necessary to re-demonstrate the feature.

So the best way is to start to demonstrate the feature and to finish with the code review.

Who?

Who must read the developer code? The technical leader? A senior developer? A junior developer? Depending on the case, every teammate must be able to do a code review. Two parameters must be taken into account to decide who can read who : The first one is the technical level of both the developer and the reader. The second one is the criticality of the feature.

For a critical feature (something complex or really important for the business), the reader must be experimented, so either a senior developer or the technical leader.

For a "basic" feature, it is better to avoid to let a junior developer read another junior developer. Indeed they may don't know every coding conventions or miss bugs or performance issues. But a junior developer can read a senior developer : it is a good way for the junior to improve his competences.

Conclusion : benefits of a good code review

A good code review allows you to fix several bugs and to refactor your code really quickly. You have so the insurance to have a best quality in your software and in your code.

But more than that, encourage your team to read the code allows them to have a best knowledge of the code and to improve their competences very easily.

So if you think that this kind of code review is a waste of time, you are mistaken because you will have less bugs in production, a code of quality, and to finish a more competent team, so a more effective team.