Preparing for Code Reviews
Podcast: Play in new window | Download (54.3MB) | Embed
Subscribe: Apple Podcasts | Spotify | Email | RSS | More
Code reviews are the process of having another developer or group of developers read, question, and review code and changes before they are promoted to production. In some cases before they even get to test be it QA or UAT. They help to ensure the best possible code is running by having multiple people look at a problem. They bring a lot of benefits to a development team and a company when implemented correctly.
Research by SmartBear has shown that reviewing code significantly reduces the number of bugs that make it to production. Developers will have bugs in their code but with reviews more are caught before it gets to the consumer. They also improve security by identifying potential risks sooner and fixing them while still in development. Reviews also allow for knowledge to be passed from more senior developers or shared among peers. Finally they improve code quality because developers are more likely to write readable code if they know it will be reviewed.
There are several ways to hold code reviews. They can be a formal process that developers have to go through to promote their code or can be an informal process to improve quality. They can be one on one with a peer or senior or in a group or the entire time. There are also several different goals you can have for your reviews. You may be the reviewer one day and the reviewed the next. Whatever your team or management decides to use these will help you to make the best of the review from whichever side you sit.
Episode Breakdown
13:25 Planning Your Time Wisely
Don’t review more than 400 lines of code at a time. Setting a line-of-code limit is ensures that you do not miss defects. The brain can only process so much information at a time. The percentage of defects found is inversely proportional to the amount of code reviewed. Reviewing 200-400 lines will find 70-90% of bugs. Study by SmartBear of Cisco Systems programers. This is the ideal amount. Setting this limit also ensures that code is written in divisible units.
Keep reviews between 60 and 90 minutes. Focusing your attention for too long will reduce your ability to concentrate. This is why we have things like the pomodoro technique when writing code. Performance starts to drop as your concentration wanes from too much use. Taking a break from reviewing will allow you a chance to rest and reset.
Review less than 500 lines of code per hour. Slower paced reviews for limited amounts of time are more effective. It can be tempting to rush through a code review to get back to coding yourself. The previously mentioned research indicates that fewer bugs are found when reviewing more than 500 lines per hour.
22:30 Creating An Environment For Healthy Reviews
Knowing someone will be reviewing your code makes people work harder to write better code with fewer bugs. This is called the “Ego Effect”. Your ego will provide both good and bad feelings. It works with both mandatory reviews and ad-hoc ones. Even just checking 20 to 30% of overall code changes will result in less bugs. Developers will be driven to double check their work if it has a 1 in 3 chance of being reviewed.
Build a company culture that encourages code reviews. Code reviews cause stress between team members. It’s difficult to have your code critiqued by your coworkers. Even more stressful is to have management looking at the bugs in your code. Managers need to build a culture that supports learning and growth through reviews. View bugs as an opportunity for growth and a way to improve code. Junior developers learn from the more senior developers. Guide senior developers to see bad habits they’ve formed over the years. The bugs and issue found in code reviews aren’t to be used for evaluations. They should never be included in individual or team performance reviews. If review metrics are a basis for promotion or pay raises then developers will begin to game the system.
Give constructive feedback on the code you review. Avoid being critical of the code written by others. Even well experienced developers are new at technology they don’t know. You may not know all the circumstances around the code. Ask questions instead of making statements. Why did you do it this way? Instead of this isn’t most efficient way to do it. There may be constraints you are not seeing or are unaware of at the time of the review. Feedback should be given in person. Intonation and inflection carry a lot of meaning when communicating. Other nonverbal cues are lost when not in person for reviews.
If you do not find a bug then approve the code even if you have suggested improvements. Start with the assumption that the developer can make the suggested changes. Only change this assumption after it is proven wrong. Second round reviews for suggested changes is not an effective use of time. Developers will become reluctant to submit small changes to code if they are held up due to issues with style such as variable naming. If you don’t approve give a reason for the lack of approval. If you need someone else to check it tell the developer so they know that you didn’t forget. This keeps open the lines of communication and may highlight areas in the process that need improving.
36:40 Creating Procedures For The Team
The process of code reviews should be simple and lightweight. There are many ways to review code with peers and leads. This can range from emails or documents exchanged to in person conferences. One tool that is nice is Live Share on VS and VS Code. Reviews can be formal events scheduled with one developer presenting their code to a group. These tend to take longer to review less code. Process can get in the way of the actual review. Lighter weight reviews take less time and cover more code in that time.
Before your reviews establish the purpose and goals of code reviews as a team. This puts everyone on the same page so that you are able to use the code review effectively. Developers, reviewers, and managers will know what to expect from the review going into it. They will also know what to do with the results of the review. Also determine how you will know the code review has been effective. Inspection rate is the velocity the review is performed. Defect rate is the number of bugs over time. Defect density is the average bugs per line of code.
Code reviews should be used for learning and spreading knowledge among the team. They should not be used for catching bugs, though you may find them in the review. Avoid using them to enforce styling rules unless this is a big issue.
Come up with a way to deal with bugs and other issues found in the review. Create a process for developers to follow if bugs are found in their code. This could be not passing the review until the bugs are fixed. Likely a second review will need to be held once the bugs are fixed. Encourage developers to not submit for review if there are still bugs in the code. Have another process for helping them if they are struggling with a bug. Be lenient as this is a learning process, especially early if you are introducing this to your workplace. Also have a procedure in place for what to do with suggestions or non bug issues found such as styling variances.
45:25 Preparing To Be Reviewed
Comment your code before the review so it is easier to read and understand. Comments guide the reviewer through the code. It’s especially important if you are making changes to an existing code base to note where you made changes. In newer or greenfield your code needs to be readable commented so that it helps with understanding. They can also prevent unnecessary questions by explaining decisions or variances from standards. This means making sure you are following any set code standards when not explicitly explaining why you are not following them.
Use automated processes and code analyzers before submitting for review. Code reviews are a manual process. There are many things that need to be checked manually in code. However some things can be automated. This will save time by finding issues before the review. Tools such as static code analyzers help to ensure code standards are being followed without having to manually view the code.
If it’s an option choose who reviews your code based on what your goals are in having it reviewed. If your goal is to share knowledge or improved skills then review with your whole team. This can be to share knowledge or skills you have gained or to get help from the team. Reviewing with a peer will allow you to reinforce your code standards and practices as well as look for potential issues. They may see things you do not coming from a different perspective. Explaining to someone requires an even deeper understanding. Choosing a more senior developer to review your code can accomplish any of these goals.
50:40 Preparing To Be The Reviewer
Have a checklist for the review so that you don’t forget anything. Checklists will help to standardize code reviews. It can be easy to be lenient when reviewing the code of people we like. Having a list of things to do or check in the review will help reduce this bias. You’ll likely find that people make the same mistakes on a regular basis. The worst of these is leaving something out or forgetting about it. A checklist of things to look for in the code will help prevent overlooking these common mistakes.
Plan the code review as soon as you are given the request. If you don’t have scheduled reviews you may not be able to review the code immediately, especially if it is a large change. At least scan through it and let the developer know you are looking at it. On the first look try to understand the change. Take notes with any questions you may have for the developer. Schedule a time as soon as you are available to review the code. Give yourself a time limit on how long to spend reviewing the code. If it takes longer than your limit schedule more time.
Check out the functionality by actually using the code or app. Reading code is more difficult than writing and much more difficult than running it. It takes a very experience senior developer to see runtime bugs in just code. Your also likely to use the app differently than the developer expected. Playing with the app or feature will allow you to make sure the code is doing what is expected. Breakpoints can reveal a lot about how the code is running in a more complicated environment. This will also show how the new code integrates with the existing application. On a similar note use the same IDE or a similar as the developer when reviewing the actual code.about your application.
IoTease: Product
Das Keyboard 5Q
This keyboard calls itself the first smart mechanical keyboard. Different keys on the board change color based on different messages like the weather, CPU activity, or anything else you want to receive notification about on your keyboard. It increases productivity through responsive feedback up to 45 times faster than standard keyboards. The keyboard also lays claim to the brightest RBG+ backlighting with 16.8M per key color. It’s built to last with Gamma Zulu mechanical switches and an anodized aluminum top panel. Built for developers it has it’s own open RESTful API that you can use to program it however you want and even connect it with other IoT devices.
Tricks of the Trade
Learn the difference between things that need to be reviewed and things that need to be penalized. The latter is less common, while the former is core to a team working well.
Links
Editor’s Notes:
This is our first attempt at remote recording. We did our best to reduce the noise, echo, and clicks. Bear with us as we learn a new process and what works now.
I really did like this episode. Just started working for a new company where code reviews are optional (in fact I did not have any official review yet, working here for about 1.5 months). So I am planning to get code reviews on our agenda but I know it is going to take a while to incorporate it in the process. Thing is, I am very used to just ask any developer around to come sit next to me to do some code review so that’s what I have been doing now. Just setting an example (I hope), show them I do not fear their opinion about my code and that I am willing to a) learn from their views and b) teach some new things I have been doing. Some developers already started doing the same, so I am getting the ground ready for code reviews. I did not get them to listen to this podcast though. Aren’t people lazy sometimes! I told them my dishwasher has been broken for about two years now so I use the time I am washing the dishes to watch pluralsight or listen to podcasts. The looks I got, just priceless 🙂
One thing I have to disagree though: why should I not let our customer(s) pay for code reviews? It’s them that benefit most: more developers know the code, less bugs, cleaner code.
Anyway, keep up the good work, enjoying your shows (while washing the dishes or riding my bike to work).
It’s not so much about letting customers pay or not. It’s putting it as a line item on the bill. That’s how bean counters get involved (the same ones that always find the spectacularly abrasive paper for the restrooms). They’ll push to cut it if they see it. Code reviews are part of coding time and should be included in the bill, for sure. But don’t call it out separately, as doing so encourages people who aren’t in the industry to try and tell you how to do your job to save a buck. It’s kind of the same thing with unit testing, refactoring, pair programming – there are just certain things that a professional developer does that shouldn’t be broken out into a line item.
The other thing this does is help present you as a professional. Pros in most industries know what they are about, and don’t break things down into very small chunks for billing. For instance, a mechanic charging hourly isn’t going to break out details on how long it took to change your headlight while they were rebuilding your engine. You want to set things up so that you have a list of tasks, do them, and get your pay. Not only does it keep you from being micromanaged, but it encourages you to become more efficient, as the results of that efficiency don’t immediately get eaten by the bean counters.
I guess I probably could have phrased that one better. Good on you to get the code review process going – I’m still trying to get mine to happen more frequently.