Code Reviews are excellent and important. They increase the quality of the code, they help the team share ideas. They allow knowledge transfer and in some companies coding standards never evolve without them.
However, code reviews can be a little problematic at first in some companies and since they involve critical feedback the developers need to have the interpersonal communication skills to pull it off. They need to be diplomatic and tactful.
Here are my tips for pulling off good code reviews:
-
- Ensure it’s not a personal attack by having a sound justification for any correction you wish to offer back. Such justifications may be “one loop instead of two is quicker” or “this function name doesn’t describe exact what it does” or “our standards suggest we should keep lines under 80 characters across”.
- I personally tell them everything is optional. It’s feedback. Your company should not be employing incompetent developers who would actively do the wrong thing rather than admit an oversight. Therefore, assume the best in them. It’s better to do code reviews than to not do them for quality reasons, so don’t shut people down. In teams with code reviews, no matter the experience of a developer – they all share on the same code base.. so the opinions of the newbies is just as important as those with experience. It’s a great way to bring the group together. To share democratic power and to allow everyone to have meaningful input. When code reviews are pleasant experiences for everyone.. people’s resistance to accepting changes drops and everyone benefits.
- I focus more on the structure of the code than individual lines. Shitty lines of code that always work are fine and acceptable (as long as they aren’t likely to randomly fail). Code that is hard to restructure however is a bigger problem so focus your energy on that rather than spending your energy debating the small things. Don’t make code reviews harder than they need to be by always having to find one thing wrong.
- I do, what I call Blind code reviews. First I read the code on my own. Coming across it for the first time.. as if I were stumbling on it and it were written five years ago. I can truly see if something is or is not well commented or clear. If I think a function could be written in a better way.. I can quietly mock it up.. and see if that’s the case without assuming it can be done better. Once I’ve compiled my list of changes I sit with them and ask the questions and give my feedback in an organised way. One thing I often see in code reviews is someone saying “I did this… so then I had to do that… oh and put a comment here”. If you’re led through the code you don’t see it for what it really is and your judgement is impaired. I may try and rewrite a whole function during the code review (when I’m my own at first), as my justification, only to discover my version is 7 lines longer.. at which point I explain I tried, gave up and congratulate them on writing it the way they did it. Blind code reviews also save you the embarrassment of saying “why did you…oh that’s why” style of code review that makes the process seem like an annoying and pointless waste of time.
- Never accuse something of being wrong. Ask if there is a reason that something turned out the way it did. There probably is a good reason. There are times when we’re switched off or misjudge things and the “woah dude why?” can be fun when you’ve got a good rapport going with colleagues.. but generally be kind and ask why. If you’re a fucking self-righteous douche with contempt for your colleagues find another job. If you have respect for them then don’t make assumptions or assume incompetence. This point is about being diplomatic. Question something without condemning the person or making them really defensive.
- Compliment anything that isn’t negative or is written how you would have written it. Score some happy points. However be honest and sincere. Don’t gloat when saying something negative. You are a team and your success is mutual and bound. People like code reviews when there work is held up a “good code”, so drop in positives to even it out with any negatives. Admitting you can’t write even a line or two better comes across as a sincere appraisal of their code.
- Be specific. Don’t say “It could be better somehow” and don’t say how exactly. Give someone a small check list they can whip through quickly and without confusion. Understand they have a deadline to finish by so if you suggest changes – make them easy to implement. Help them succeed! In fact, “How to win friends and influence people” is a great book and one chapter is called “Make the problem seem easy to correct“. Don’t turn their success of completing the work into a failure… that feels personal.. like you’ve become an unnecessary obstruction and it’s getting that person into trouble.
- Ensure the coding standards are well justified and refer to specific parts if possible. Then also point to similar implementations if possible so people have an easier time understanding what you mean or how to do it. See someone edit a list items in javascript for a combobox similar to another one? Point to that implementation. This advice again falls under “help that person succeed”.
- The coding standard is advice and rarely fit for every scenario. Allow people the opportunity to evolve the coding standards if they need to and amend it where it needs amending. If you argue over a conceptual difference then once resolved put it on the coding standard to shorten the same discussion next time.
- When doing a code review, give the submitter your time and concentration. Don’t be off somewhere else. Don’t downplay the importance of code reviews. Either you as a team are all in, or you aren’t all in. You’re getting paid and it has genuine benefits you agree with.. so don’t pretend it’s a chore. If you’re busy, let someone else code review it.. but don’t do a half-assed job. Also, if you ask someone to make a change then be available for them if they are stuck implementing it.
- Something else, straight out of the book “How to win friends and influence people”: Start in a friendly way. “This won’t take long because I rarely have problems with your code. I just noticed two things..”. “I can’t believe I have a recommendation on your SQL since you’re our SQL guru..”. “90% of this code is great… just one small point”. “I know this code will work fine…” Don’t exaggerate the problem, make it seem small. Also stolen. Replace the word BUT with the word AND. “This all good AND there is just one small change”
- You can often avoid causing discomfort in developers, when you’re sitting next to them, by merely suggesting the issue and perhaps giving them the opportunity to suggest their own solution. To let them own the “newer, cleaner” implementation of a piece of code. That way they still own everything. You need to have your legitimate solution ready in case they can’t see it.. but sometimes just point something out is enough to let them realise it and fix it for themselves. You don’t need to dictate. You don’t want to be the guy that says “give your keyboard and watch me fix this” (unless of course you’ve a good rapport with the other developer. In fact, you can say “watch the magic” and make code reviews fun if the friendship is there).
- Don’t mix code reviews with testing. I can’t find a way to avoid making the other people feel bad when I have to say them “Also, this flat out doesn’t work”. It’s just too much bad news. Have a third person do the QA.
- If you think you’re a better coder and it was a damn good thing you reviewed the code before it went into production then remember that. Try and help the code reviewing process be successful. Do it by being fair and legitimately helpful. It’s mutually beneficial for you as it keeps you in the loop of what’s going on! Work on your people skills.. always ask people if they agreed with your code review advice. Ask if they think the code now is of a higher quality. Discuss the points clearly and reduce your workload for next time. Engage other developers in it. That way you actually get more say over what does and does not go into your precious software product. Try and make it painless so you’ve more impact on the system and its success.
Hopefully my tips will help everyone get along better and hopefully more companies can adopt code reviews. If you have any other ideas on how to improve the code review process I’d be interested to hear them. Remember that your people skills should be sharp when code reviewing and that those interpersonal skills are very important to your own success.