One of the most important (if not the most loved) parts of being a professional software developer is the code review. While not always a popular task, the code review has benefits to the person doing the review, the person whose code is being reviewed and the wider development team and business. In this article I’ll go what a code review is, why we should be doing them and the forms this process can take. I’ll also be providing a little advice on how to perform the roles of either code reviewer or code reviewee (and yes, I’ll be continuing using the term ‘reviewee’ to refer to the person whose code is being reviewed whether my spell-check likes it or not).
What is a Code Review?
A code review is when a software professional reads code written by someone else to check that it’s of suitable quality for inclusion on the code base, although the benefits are event greater than this implies. This will normally be a peer code review, where the reviewer will be another developer from the same team and it isn’t necessarily the case that the reviewer will be more experienced than the reviewee. Code reviews work best when all developers in the team get the opportunity to fulfil both roles. Some teams think that code reviews should only be done by the most experienced members and some developers can be reluctant to review the code of a more experienced developer but everyone makes mistakes, has lapses in concentration or even just completely misunderstands what’s being asked of them from time to time so having a second pair of eyes to check that you haven’t inadvertently done anything stupid is always useful! Also it can be a fantastic learning experience to see how a more experienced developer will tackle a problem.
Why Code Review?
As I mentioned in the opening paragraph, there are three beneficiaries of the code review so I’ll take each one in turn:
The code reviewer
The most obvious benefit to the code reviewer is the chance to ensure that other developers aren’t checking in poor quality code into your code base (which you might be required to support, or that might introduce bugs that cause problems with something you’ve written!). Of course, this is important, but the benefits actually go way beyond that. By getting a chance to see code written by other members of your team, perhaps in an area you’ve not worked on, you get to increase your understanding of the code base as a whole, making it easier for you if you’re ever called on to work in a similar area. You also get to see what new features or bug fixes are going into the code which helps keep your knowledge of the overall system up-to-date. Additionally, and perhaps most importantly, you get to learn programming techniques that you might not already be familiar with – I often first learn about new features in the language by finding some code in a review that I don’t think should compile! This is the case whether you’re reviewing the code of someone more experienced than you or less experienced. I’ve yet to meet the developer who’s so experienced that they’ve learnt everything there is to know about software development, and if they’re out there, imaging how boring life must be for them! If you see a feature or technique you’re not familiar with, you can spend a little time researching it on the internet, and if you still don’t feel you fully understand it, this is a great opportunity to get the person whose code you’re reviewing to run you through how it works – after all you can’t properly review code that you don’t understand!
The code reviewee
For the code reviewee the code review acts as a safety net. You can massively reduce the risk of an embarrassing mistake you’ve made making it into the code base by having a second person go through and check your changes. It also keeps you honest – while this might not always feel like a benefit, knowing that your code will be reviewed removes the temptation to commit hacky quick-fixes such as copy-paste inheritance or breaking encapsulation which would lower the quality of your code base and almost always come back to bite you sooner or later. As well as being a learning experience for the reviewer, the code review is also a great learning experience for the code reviewee, as the reviewer might be able to suggest a better way of achieving something, or may know of an existing function that would make the code simpler. They might even be able to identify bugs in the code, scenarios you might not have considered, or raise concerns about how the code changes will interact with something another team members is working on, or is planned to be worked on.
The business / the rest of the team
For the wider business, and the rest of the development team, the advantages of having a code review process in place are that fewer bugs will be introduced through a poorly-thought-out change, the quality of the code base will be higher and the code base will be more internally consistent. In general, it’s going to be easier to work on a code base that’s non-optimal but consistent than one where every section is more-or-less optimal, but optimised for different things based on which developer wrote it (or how they’re feeling on the day it’s being written or the last code quality article they’ve read). As mentioned earlier, it’s also a great two-way learning process, ensuring that new techniques and best practices filter through the entire development team, increasing the knowledge of the department and therefore the quality of the code that will be written going forward.
How to code review
In my experience, the most common mechanism for performing a code review is through the use of a tool that allows you to post your code changes to a central location, ideally before they’re committed to your main code repository but sometimes afterwards. The tool typically then notifies reviewers that there is code that required attention and provides a mechanism to view the code and comment on it, usually giving then option to give an overall ‘thumbs up’ or ‘thumbs down’ to indicate whether the code is considered good enough. The reviewee is then notified that they have comments on their code which they should review and perform any corrective action necessary. There are a number of companies providing code reviewing tools that work like this, either built into the code repository such as Atlassians’s Bitbucket, or as stand-alone tools like SmartBear’s Code Collaborator (other products are available). They vary in how well they integrate with the rest of the development process, as well as the number of features and how easy they are to use, but basically all do the same thing.
An alternative, is the ‘meeting style’ code review where a meeting room is booked and the code reviewee is invited to present their code to one or more reviewers in the same room. This can be a little more intense for the reviewee, especially if run poorly as they may feel under attack and is also less time efficient, as both the reviewee and all the reviewers are blocked out and unavailable for the duration of the review. The advantage of this approach is that a proper conversation can be had, ensuring that the reviewer understands why the code was written in a certain way, and the reviewee understands why any changes are being suggested. It may also be possible to make any changes in the same meeting as a pair programming exercise so it can be a good option if the changes being reviewed are for the most urgent task the team has on that day, or if the code is particularly complex or in area that is prone to knock-on bugs when changed.
For an effective code review it is very important that both code reviewer and reviewee go in with the right attitude. Reviews aren’t about who’s the better coder, they’re not about scoring points and they’re definitely not about making people feel bad. They’re about improving the quality of the code base and sharing knowledge, which is good for everyone.
For the code reviewee:
Be humble. It can be painful to have spent hours lovingly crafted a block of code that you think cleverly and beautifully solves a particular problem, just to have someone else tell you that it’s no good, but everyone makes mistakes, and we should always embrace an opportunity to become better at our role.
Try not to take comments personally. Remember that any criticism that comes is criticism of the code, not criticism of you as a person. Likewise, while your code change might be marked ‘rejected’ that isn’t a rejection of you or your abilities, it just means that the code could be better, and that you’re the one who’s going to make it happen!
Listen to what you’re being told, either in person or through comments on the code review tool. If any changes are suggested then it’s likely that they don’t only apply to the code currently being reviewed, but that any code you write in the future should be written in the same way so ensure that you understand them and undertake to apply them going forward.
Take the opportunity to learn. If the change is related to the company’s coding standards take the opportunity to re-read the relevant section. If the change suggests an alternative coding technique or language feature read up on that. If there’s anything you’re not sure of, or don’t understand, get the reviewer to explain it to you.
Don’t be afraid to push back. This should never be a knee-jerk response and you should never be in the mindset that your code is so perfect that any suggested changes must be wrong and/or indicate that the reviewer isn’t clever enough to understand your code. However, everyone makes mistakes, even code reviewers, and if you’ve listened to and understood the change that is being suggested and you don’t think the change is required, or will be detrimental, explain this to the reviewer and why you think that. It may be that they’ve missed something, or that they’re unaware of a quirk of another part of the system that requires code to be written in an unusual way and when this is explained to them they might retract the suggestion. If there’s a deadlock and after discussing and understanding each others points of view the reviewer still feels that the code should be written one way, and you still feel that it should be written a different way, you should generally go with the reviewer’s suggestion. This indicates that actually either way is fine but the process makes more sense if the reviewer is given the deciding vote.
For code the reviewer:
Be humble. Just because you don’t understand some code, doesn’t necessarily mean it’s wrong! Perhaps it just indicates that a technique or language feature is being used that you’re not fully up-to-speed with yet.
Don’t be afraid. Just because you don’t understand some code, doesn’t necessarily mean it’s right! Even if you’ve only been with the company for a few weeks and you’re reviewing the code of a 10 year veteran, it’s possible that you know something they don’t, so if you’re not sure about something flag it up. If they have a good reason why the code is written a certain way then that’s great, you’ve learnt something. If not, they’ve learnt something and you’ve increased the quality of the code base.
Remember that the reviewee is a human being. Human beings have feelings and while we may try to hide it sometimes, no-one likes it if those feelings are hurt. Unless your relationship with the reviewee is very good and you know they won’t be offended or take it personally, avoid unhelpful comments insulting the code, or even worse the developer who wrote it. It’s just as easy to be kind and say ‘I think there might be a problem with this line’ or ‘Have you considered …?’ and it gets exactly the same results as ‘This line is stupid’ or ‘You forgot about …’. It’s also less embarrassing to have phrased things this way if it turns out that in fact the code was fine and in fact it was your knowledge that was faulty!
Personally, I think that code reviews are the single best way for software teams to maintain the quality and consistency of their code base as well as increasing the knowledge and understanding each member of the team has of the languages being using and the systems being developed. If you’re working in a department with more than one software developer and you’re not performing code reviews on a regular basis, you should be asking why not? Hopefully this article has helped you understand the important of code reviews and why they should be viewed as an opportunity rather than a chore.
