DEV Community: Michael Müller

Code Reviews: Benefits, Misconceptions and Challenges

Michael Müller — Wed, 19 Jul 2023 15:46:55 +0000

Code reviews have become an industry standard and are part of the development process at companies ranging from small startups to large corporations like Google, Facebook or Microsoft. According to a 2019 Stack Overflow survey approximately 76,4% developers review code at work. While they must provide some obvious benefits to have achieved such a broad adoption, not every developer is a fan. Some consider them a roadblock and hate them with a passion.

In this post I want to discuss the real benefits of code reviews and find out why they don't seem to work in every team and what you can do about it. To dig into this topic, let's start with a little history on how code reviews have become a thing.

History Of Code Reviews

The first ideas for code review processes emerged in the year of the Apple I release and the introduction of 5.25 inch floppy disks. In 1976 Michael E. Fagan published the article "Design and code inspections to reduce errors in program development" in the IBM Systems Journal, Vol. 15. It describes a process for preventing defects in the final product by reviewing the design of the software as well as the actual code. The code inspection is performed by a team consisting of 4 members with different roles (a moderator, a designer, a coder and a tester). They review each and every instruction at an estimated speed of 150 lines per hour and a maximum session duration of 2 hours. The final output of this process is a written report like the one below.

Sample code inspection report from the article "Design and code inspections to reduce errors in program development" by Michael E. Fagan, published in the IBM Systems Journal, vol. 15.

There is not much evidence that code inspections received a broad adoption in the industry. The most obvious reason would be that formal code reviews were too time consuming. In the report shown, it took 27.3 person-hours to review 348 lines of code, which would be insane by today's standard. We have to keep in mind though that the tools and programming languages used nowadays differ significantly from the ones in 1976. For example, the check list provided in the article contains many items that would be caught by modern compilers/linters (e.g., "1. Are All Constants Defined?") or are no longer relevant (e.g., "8. Are Registers Being Restored on Exits?").

Modern software based code reviews, also known as informal or peer code reviews, work very differently. Instead of choosing a point in time when the entire code base is reviewed, changes are reviewed before they become part of the code base. Both parties do not sit together in one room anymore, but work asynchronously and leave comments for each other in a review system. The first tools to support such a software based review appeared around 2003 as desktop applications or plug-ins for IDEs and were later largely replaced by website-based solutions. The basic principle hasn't changed much since then.

Advantages of Code Reviews

When talking about the benefits of code reviews many developers think about reducing the number of potential bugs that reach the production code. This was also the original motivation for the invention of inspections in 1976:

The inspection is not intended to redesign, evaluate alternate design solutions, or to find solutions to errors; it is intended just to find errors!
Michael E. Fagan

Scientific research in this area has shown that the motivation and benefits of modern code reviews are different today. The study Code Reviewing in the Trenches: Understanding Challenges, Best Practices and Tool Needs from 2016 analyzed the code review process at Microsoft and discovered that finding defects is only the second most important motivation. A similar Microsoft study Expectations, Outcomes, and Challenges
Of Modern Code Review 3 years prior also found other benefits in addition to identifying defects. This effect is not limited to Microsoft, Google also did some research and published Modern Code Review: A Case Study at Google in 2018 describing similar results at Google.

Let's take a closer look at some of the ways you can benefit from code reviews.

Code Improvements / Better Solutions

As a software developer, you must constantly educate yourself to keep up with newly developed frameworks, tools, or even languages. The way software was developed ten years ago is very different from today. As a result, an individual developer can never know everything and may not always be aware that there are better solutions to a problem they are currently solving. Code reviews are a great way to point out small simplifications or improvements that will greatly improve the maintainability of your code.

Finding Defects

The main reason code reviews were invented was to find bugs. Nowadays, the importance of this motivation can vary greatly depending on the project you are working on. If your team is writing software for an embedded device that can't be updated remotely, finding potential bugs is much more important than for a website that can be deployed in seconds. In recent years, this aspect has become less important, as more and more errors can also be found automatically. In addition, there is a growing tendency to focus on the conceptual structure of the changes, and not necessarily on every little detail of the implementation. Regardless of the priority this aspect has in your team, it is certainly better to find bugs at this stage than to have a customer complain about them.

Knowledge Transfer

This is perhaps one of the most important aspects of code reviews, especially in small teams. As part of the process, the reviewer needs to understand the code written by someone else, figure out the motivation behind it and perhaps even start a discussion to understand further details. This transfers knowledge from the code author to someone else on your team and prevents the author from becoming a single point of failure. If a developer is unavailable (vacation, illness, ...) and a bug in one of their components needs to be fixed urgently, there should now be at least one other knowledgeable person for that area. This greatly reduces the risk of adding more defects when making changes to existing code.

Maintaining Norms

Every code base has its own set of norms, whether in the form of style guides or implicit rules about how certain problems should be solved. Some of them can and should be enforced by automated tools, but many of them are too complex and require a human review. For example, when you are working on a project for a long time you will notice that code consistency is one of the key points to achieve maintainability. While it is usually impossible for software to compare the intention of two code fragments, an experienced developer can often easily tell during a code review that you're reinventing the wheel.

Team Awareness / Share Ownership

Working on a mission critical component can be stressful, especially when real world testing is difficult. When you do code reviews you know that at least one other person is looking at your changes and may catch bugs that you didn't notice. It is also a great way to interpret code as a result of teamwork and to be less protective of it.

Cartoon "Good Questions" by Geek & Poke, provided under the CC-BY-3.0 license.

Misconceptions about Code Reviews

We have discussed several benefits of code reviews and how they can help improve your development process: Your software has fewer bugs, is easier to extend, and all developers are more familiar with the code - thus making it less problematic when developers in key positions leave a team. If there are so many benefits to performing code reviews, you may wonder why some developers hate them.

In my experience, negative opinions about code reviews usually arise when developers try to use code reviews to solve problems that would be better addressed in other ways. If you expect your reviewer to verify tiny details in your code that could be checked by automated tools, your team will have an unpleasant experience and dislike code reviews. The focus of code reviews should be on the logic and maintainability of the code.

Here is a list of some things that should be handled outside of the review process or that you should be aware of.

Preventing Build Breakage / Test Failures

A reviewer should not have to stop their work only to find out that the code does not compile or that the tests fail. You should either set up a CI that builds the software and runs the tests or use pre-commit hooks to prevent broken code from being committed. Maintaining a comprehensive set of unit and integration tests can also catch bugs early and provide some confidence that the code meets the design requirements without forcing the reviewer to manually review each case.

Styling/Formatting

There are many different code styles and some developers tend to have strong opinions about them. You should try to decide on one and then use tools to format the code / enforce the style. This can greatly reduce the number of revisions as each change may introduce a new style violation. Nitpicking about style issues can also be used as an excuse not to review the actual logic. If using automated tools is not an option, try instead to add a rule that you focus on other aspects of the code before commenting on style issues.

Linting

Every time a reviewer finds an issue in the source code, two team members are interrupted in their work. The author of the change must pause their current work, read the comment, checkout the corresponding branch, fix the issue and push the changes. Then the reviewer has to open the new revision and verify the fix. These interruptions can have a significant impact on your team's efficiency. Tools that can catch certain classes of bugs (such as linters) before the reviewer starts analyzing the changes are a great way to reduce the workload. If you encounter recurring issue patterns in code reviews, you may want to investigate whether you can configure or extend existing linters to detect them automatically.

Not All Team Members Participate In The Process

To get the most out of your code review process, everyone on the team should participate. Skipping code reviews for the team leader or limiting them to junior developers prevents knowledge sharing and sets the wrong mindset. New team members can come up with fresh ideas during a code review and senior developers will still introduce bugs from time to time.

Challenges

Code reviews can greatly improve your processes and teamwork, but that doesn't mean they're without their challenges. I think the two most common ones are:

Code Reviews Are Time Consuming

Reviewing code will always take some time and cannot be fully automated. Tools may find logical errors, but they cannot, for example, check whether an implementation meets the requirements. If reviews tend to take forever, this is usually a symptom of other problems. Possible causes include changes not being split into proper commits, disagreements about certain processes / coding standards or unclear requirements. While these issues become very visible in code reviews, they also have an impact on the maintainability of your code. So I would suggest digging a little deeper if your code reviews are taking too long.

Tension Between Reviewer and Author

The reviewer acts as a gatekeeper and criticizes the author's work via text messages. With this type of communication, there is always a risk that the other person will misinterpret statements and that comments will be taken more negatively than they were intended. This can easily lead to tension and hurt feelings. If the code requires major changes or the discussion gets too heated, you may want to move to a face-to-face meeting or video call. This makes it easier to find solutions and avoids the "author vs. reviewer" mentality. As a reviewer you can also try not to focus only on the negative parts...

Cartoon How To Make A Good Code Review by Geek & Poke,
provided under the CC-BY-3.0 license.

Conclusion

Code reviews are a great tool for improving code quality, sharing knowledge and fostering teamwork. It is therefore no wonder that they have become so widespread. However, whether you actually get these benefits depends on how you implement code reviews. You should try to automate all the boring aspects (run tests in a CI, validate code style via a formatter, use linters, etc.) and let developers focus on whether the code meets the requirements and is maintainable.

Code reviews also tend to uncover other issues in your development process. If you are having problems with your review workflow, you may want to dig a little deeper and find out if reviews are the real cause.

SemanticDiff - Language aware diffs for VS Code

Michael Müller — Mon, 12 Dec 2022 19:18:45 +0000

What I am building

SemanticDiff is a programming-language aware diff for Visual Studio Code. The extension helps you understand code changes faster by hiding style-only changes, detecting moved code blocks as well as refactorings.

I think it is easier to understand using the following image:

If you want to see the features in action, try out the online demo. Simply select one of the existing examples or enter your own code.

Why I am building SemanticDiff

Most diff tools like git diff detect changes between two versions of a code by comparing each line character-by-character. While this works well in many cases, it can produce a lot of noise if you reformat your code or perform other types of refactorings.

For example, splitting the parameters of a complex function call across multiple lines produces a diff that isn't very useful. It looks like you have completely replaced the code:

- verify_token = generate_token(user, models.TokenType.EmailVerification, datetime.timedelta(days=2), email=user.email)
+ verify_token = generate_token(
+     user,
+     models.TokenType.EmailVerification,
+     datetime.timedelta(days=3),
+     email=user.email,
+ )

You would now need to manually compare all the parameters to spot that I extended the duration from 2 to 3 days. Wouldn't it be great if your diff tool could filter out all the irrelevant changes, so that you can immediately see the changed parameter?

That is what SemanticDiff does. It makes reviewing code less tedious and more secure. By hiding style-only changes and highlighting modifications within moved code blocks, you are less likely to overlook anything important and you have to review less overall.

How does it work?

SemanticDiff implements a pipeline consisting of three stages:

1. Code Parsing

The old and new code is parsed into an abstract syntax tree (AST). These trees contain all the information that a compiler or interpreter would need to compile or interpret your code. This approach gives us two advantages over the original text representation:

We have additional information about the meaning of individual characters (e.g. that generate_token is the name of a function that is going to be called).
All characters that don't have an effect on the program flow, like white-space or line breaks outside of strings, are automatically filtered out.

2. Tree Matching

The nodes of the old and new tree are matched to identify which parts of the code have changed and which are still the same. This involves comparing all the nodes of the old and new tree to find those that are identical.

Since we know the structure of the code, we can implement a more advanced comparison than a normal text diff. For example, the following three python statements look quite different when comparing the code character-by-character, but it is easy to verify that they are identical using the AST tree.

a = "Hello\nWorld"
a = 'Hello\n' \
    'World'
a = """Hello
World"""

3. Text Diff Generation

The last step is to create a side-by-side diff so that a developer has an easy way to understand what has changed. This involves aligning the old and new source code using the generated mapping. All old nodes that can not be found in the new tree are marked as deleted in the diff and vice versa for new nodes.

Since we allow mapping any pairs of nodes across the two trees, some parts of the code can not be aligned properly. This occurs for example, if a block of code has been moved. Special handling for these cases is required.

Want to try it out?

SemanticDiff will soon enter closed beta. If you are interested in better diffs integrated directly into VS Code, join the waitlist. You get a notification email as soon as the beta starts.

In the meantime you can play around with the online demo.

Also let me know if you want to know more about the inner workings of SemanticDiff 😃️.