Code Reviews

I’ve been meaning to blog about code reviews for a while, but I’ve never seemed to sit down and organise my thoughts on the matter. Well, I’ve got some time now, so I’ll do my best to organise my thoughts.

First things first – Code Reviews are, in my opinion, not a “nice to have”. They are an essential part of the quality control for software. Anyone that says they are a “nice to have” simply doesn’t understand the ancillary benefits of doing a code review besides ensuring everyone is keeping to the same style. In fact, keeping everyone to the same style is to my mind a pretty minor part of code reviews.

Whose code should be reviewed? Everyone that writes code should have it reviewed. From the gap year students up to the senior architects (if they still write code). There is no level where you say “That person is senior enough to review their own code”. I am a fairly senior developer and I am not so arrogant to think that I won’t benefit from code reviews.

Who should conduct code reviews? Everyone should conduct a review. Now, obviously the graduate developer isn’t going to pick up on things that a senior developer might, so this is more of an opportunity for the graduate to read someone else’s code. They will, hopefully, learn more from reading a senior developer’s code than they will find fault with it. It affords them the opportunity to ask questions regarding why things were done in a certain way. And often a graduate will bring a fresh perspective to things by suggesting new techniques that the senior developer may not have had time to read up on yet, but which the graduate recently learned at university.

For pretty much everyone on the team, conducting a code review means looking through and understanding code in parts of the system they may not have the opportunity to see otherwise. This means that the developers on the team will have a greater understanding of the overall system. Maintenance, once the system is live, becomes easier to manage because all the developers that worked on the system should know enough about all its parts to be easily assignable to the task. Management doesn’t have to pull a specific person off another project and if a developer moves on there is still retention of knowledge about the code in the company.

If code reviews are conducted frequently it gives the reviewer a chance to ask the developer about design decisions while there are still fresh in their memory. The reviewer can potentially learn about new techniques, or potentially head off future problems if the decision was incorrect.

Code reviews are difficult when developers’ egos become involved. It is therefore vitally important that reviews are constructive. It is not sufficient to point out where something is wrong, but to suggest a way to improve it as well. As was mentioned earlier, the reviewer should find out why the code was written in a particular way. The developer may have a valid reason that the reviewer was not previously aware of. It is better to find that out first before criticising the developer for seemingly poor code. That said, it is important to find out what alternatives they considered and why they think their ultimate decision was correct.

Positive aspects in the code must be found also. If a developer constantly receives poor code reviews they may become protective of their code and refuse to submit it for review, or otherwise indiscriminately attack other developers on the team or management rather than seek to address the problems that exist. If the developer under review is junior they will most likely deliver poor code to start with. With each code review comment on areas that they have improved on so they know that they are getting better as it is often the case that as one area in need of improvement is resolved other areas become apparent.

It is also crucially important that reviews are frequent so that problems do not build up. The worst thing is for problems to have built up over a number of months and for a developer to only find out about it during their annual review. This will lead to resentment.

In short, everyone’s code gets reviewed, and it gets reviewed often.

Tags: code review software development quality