Anytime we add new, complex code to our project, we’re setting a trap for our future selves (and for our colleagues). Every time we have to deal with that complex code in the future we’re going to be paying a tax on our productivity from the added time it takes to work with that code. And the more complex our code is the more likely it is that we are going to accidentally introduce bugs into our project in the future.

Ideally, everyone would just never produce complex code to begin with. But we all know that’s not realistic 🙂 - so the best thing we can do is make sure we’re catching unnecessarily complex code before it’s merged. Today I want to look at how we can catch complex code during the code review process.

Two types of complex code - essential and accidental complexity.

Before we dive into how we want to catch complexity during reviews we need to acknowledge something about complex code.

Not all complex code is made equal. Some code complexity is absolutely necessary because we’re using code to solve complex problems. This is essential complexity - it's the complexity needed to get the job done.

On the other hand, there's complexity that we introduce into our code that is not necessary. It can be eliminated from our code and we would still get the job done - this is accidental complexity.

When we think about catching complex code in reviews we just care about the accidental complexity - complexity that we don’t need to have in order to achieve the goals of the code.

Identifying complex code

There are a host of ways we could try to identify overly complex code being introduced in a PR - but I tend to think there are 4 things to look for that will catch the most harmful additions of excess complexity to your project.

1. Recreating Existing Functionality or Creating Duplication If newly introduced code duplicates existing functionality in our codebase, it strongly indicates we can reduce the code's complexity. ****We can reuse the existing functionality in the new code or abstract it out for more general usage. This may also indicate that the new code was not thoroughly planned or that the developer is not fully familiar with the codebase.
2. Lacking Decomposition When there are chunks of code that are trying to do too much it is putting us at risk of future issues from code complexity. If I'm reviewing code and seeing functions, classes, or modules that are trying to do too much at once then I will immediately flag that as an area of complexity that needs to be reviewed and reworked. By breaking down the code into smaller sections with distinct purposes and responsibilities we can make it easier to understand, easier to maintain, and easier to build on in the future.
3. Deep Nesting or High Cognitive / Cyclomatic Complexity The more nested the code is, the harder it is for anyone to understand. This is a visually easy to flag version of complexity and increased cognitive complexity has a direct impact on slowdowns of future development. If you see cases where the code is too nested then there is a clear need to rework the code before merging it.
4. Over-engineering The first three types of complexity I’ve described are relatively straightforward to flag and identify. Determining whether or not code changes are over-engineered is a bit trickier and requires you to have a good understanding of the code base and the goal of the code changes. Assuming you have that knowledge, you want to check to see whether the code changes are achieving those goals and doing it in the simplest way possible. A typical sign of over-engineering is looking to unnecessarily future proof new functionality or cover use cases or edge cases that may never occur or that weren’t needed.

You can look for other types of complexity during the review process, but if you’re able to check for these 4 aspects you should start cutting down on unnecessary complexity sneaking into your project.

Catching complexity with Sourcery

We’ve set up our Sourcery code reviews to specifically check for unnecessary code complexity as a part of the review process (if you want to learn a bit more about how we do it under the hood we’ve written about it here).

We look for the 4 types of complexity risks I’ve described above, as well as a few others, and then help you understand how you can fix any unnecessary complexity in your code changes before you merge. This saves you from leaving any traps for your future self.

---

We’re working to build a better automated version of code reviews at Sourcery. Try it out on any of your GitHub repos or reach out to hello@sourcery.ai to learn more.