Why You Should Care About Static Code Analysis
Photo by rawpixel on Unsplash
At school, we’re usually taught how to make code work, not how to make it last. This is an incredibly important distinction, since the employer at that first job you nail needs code to last, but executives and business people don’t really understand how that happens. Unless you’re lucky enough to have a great mentor to show you the difference, you’ll need to learn the hard way… or will you?
At university, I was given an assignment to programmatically determine a set of facts about a piece of C code. How many functions does this have? How many comments, and how many statements are there? How much white space does the file contain? Years later, I joined Solidware as their interim VP of Engineering and first employee, and — back in the day when Adobe Flash was cool — we took it to the next level with Splat, eventually evolving to SWaudit, where we could predict how “risky” a piece of code was based on a set of analyzed qualities: sure, code might run, but how safe is it to modify?
Programs must be written for people to read, and only incidentally for machines to execute. — Hal Abelson
Essentially, there’s a bunch of commercial and open source Static Code Analysis software out there that measures various qualities of code as they pertain to humans, and these measures can provide some remarkable insight into the well-being of your product.
I’ve covered a few of my favorite qualities in this article, but there are many others.
Have you ever scratched your head wondering what that weird named function does? Having variables in a block of code named i, j, k, and p for no apparent reason, and without explanation will likely not endear you to your successors.
For symbols (such as variable and method names), we tend to think of as longer being better, although this is a bit of a generalization. Making the intent of symbols easy to understand will make the life easier for those coming along behind you. It’ll also help take you further on the path of Self Documenting Code.
Content Length (Halstead Length)
Have you ever looked at a function and got really scared? You know, the ones that are three or four pages long and you have to scroll so much that it takes you ten minutes even to work out what’s going on.
Method (or function), class, and file content become harder to understand as they grow. The bigger these things are, the harder it is for human beings to keep implementation details clear in their head while trying to reverse engineer what you wrote. There was an old saying that an entire function should fit on the screen, but times change and perhaps better to say you can read out the thing in a single breath.
As an illustration, I once ran something like
find . -name '*.cc' | xargs wc | sort -n on a highly successful commercial embedded application. I found that one source file in particular was 175K in size, and contained hundreds of classes! Over the next couple of years, that file grew a further 50%. Arguably there are clearly some architectural and oversight concerns here, but above all, I hated the burden of learning for software developers who were new to the project.
You look in horror at the method that has multiple levels of nested loops and conditionals. You spend time working your way through, but… nope you misunderstood a particular corner case, and missed a critical bug.
Thomas McCabe did some research in the 1970s where he measured the number of distinct pathways (meaning decision points and conditional loops) through a particular piece of code.
The purpose of software engineering is to control complexity, not to create it. — Pamela Zave
He did this because he realized that the more complicated an implementation, the harder it is for people to understand what’s going on. If that code is functionally correct then of course it will work, but changes will become increasingly brittle as the code is modified and the number of distinct pathways grow. Successful unit testing also becomes harder as the number of these pathways grow.
The solution? Make sure that you have absolutely no more than ten or so (personally, I hate more than five or so) decision points within a function or method, and don’t delay refactoring and abstracting as that number grows.
Have you heard of spaghetti code? Have you ever tried to follow your way through dozens of seemingly unrelated bits of code in order to get to that one function you need to change? Have you ever tried replacing one piece of functionality in an application with a new implementation?
Unless you have the simplest piece of software, implementation of complex problems usually requires taking a requirement and decomposing it into a number of logical blocks, perhaps iteratively. So coupling is inevitable.
The trick is to design systems in such a way that they maintain modularity and loose coupling.
There are two types of coupling, and each have their own implications. I use the word component here, but it could apply to method, class, package, or even service.
Afferent coupling (Ca) measures how many components depend on a given component. It’s a metric that can be abused, but a high afferent figure implies that there’s a high degree of code reuse. Which is awesome. Certainly, you’d expect some components to be called a lot, such as authentication, authorization, and eligibility, logging and metrics, and page rendering (amongst many others).
Efferent coupling (Ce) measures the number of components on which a given component depends. So if your component has a high efferent coupling, then it’s more likely that changing one of those downstream components will break your component.
There’s also the instability index which is calculated as Ce / (Ce + Ca). High instability occurs when a component has lots of both upstream and downstream dependencies, and typically works as a great conduit for bugs to ripple.
Overall, these metrics hint at the underlying architecture, and require some interpretation. If a handful of components have high afferent coupling, and a few others have high efferent coupling, then you’re probably in good shape. However, high afferent and efferent figures across the board almost certainly means that your code is plagued with spaghetti issues.
Have you ever seen a class that seems to do everything, with dozens of fields and methods? Sometimes it’s fine, but sometimes there’s opportunity to better segregate into several classes. The more aligned the fields and methods are with one another, the easier the class will be to comprehend.
Always code as if the guy who ends up maintaining your code will be a violent psychopath who knows where you live. — John Woods
Here’s a specific example. You’ve got this class called Product Inventory which describes an inventory of products in a warehouse, and you’re coding away. You’ve added methods for checking stock levels, for adding and removing products, and so forth.
At some point, you realize that a product inventory is related to the warehouse that houses it. because you’re in a rush and not thinking about it, you start to add some methods to manipulate warehouse details to your class.
But really, warehouse and product inventory are separate concerns, and you’ll end up with one set of methods that operate on inventory related fields, and another set of methods that will operate on warehouse detail. It would work better to create a separate Warehouse class to contain that other stuff.
Cohesion refers to the degree to which the elements inside a class or similar collection are related. In the above example, we’d score pretty poorly because there are two distinct sets of fields and operations.
This is important, because by allowing this behavior to continue, you’ll end up with a Swiss army knife class that has a bazillion unrelated fields and methods and tries to do everything.
There’s more nuance to this, but thinking about how you design classes with the data that they operate on is a huge start.
So… you’ve got this class Do Something, and you need something that does something similar, but with some minor differences. If you really want to get a result quickly, then the fastest thing you can do is copy that class to perhaps DoSomethingPlus, make the modifications you need, and bam! All done. Your boss will be amazed at how fast you made the change.
Now, skip forward six months.
Perhaps one of your customers finds a bug, or perhaps one of your teammates finds a security hole in DoSomething. The engineer assigned the work fixes the flaw quickly, and life goes on. But they weren’t involved in your work and aren’t aware of DoSomethingPlus. The problem is not truly fixed, and the company still has exposure.
Isn’t that at least a bit scary? Fortunately, CPD tools can report on how similar various blocks of code are, and highlight those that appear related.
IDEs Aren’t Always Your Friend, but Continuous Integration is
Modern IDEs help the developer a lot: features like find references, the ability to skip back and forth through package lists, and the ability to collapse functions all assist in navigating an increasingly complex world of source code. But be cautious of this and the inevitable escalating arms race which makes it easier to make bad choices during the development cycle.
Maybe your IDE has some tools to help you manage complexity, but make sure there’s a safety net in either CI or an automated gate that runs prior to someone taking a look at the merge so that your tools will automatically highlight when changes begin to drift outside of acceptable ranges. Most recently, we used SonarQube because it has support for a broad range of languages, contains all sorts of rule sets, and teams could compare results over time as well as cross-project. But you have plenty of options.
Soooo… How Does All This Apply to DevOps?
In order to keep velocity high and problems with code low, it’s important to fail fast and get feedback on potential problems early. This particular class of problem will never be found through traditional functional or integration testing, yet leaving bad decisions to fester and compound will have a profound impact on software quality and engineer morale over time.
Faced with the risk of breaking some old obscure code that nobody understands, it’s easier to just throw that code away, or build a layer of abstraction on top. Neither of which are great options.
It’s harder to read code than to write it. — Joel Spolsky
But it’s not just about individual engineers honing their software craftsmanship skills: as a technical or product manager, I want to have confidence that we’re investing our money in the right solutions and not making decisions that will hamstring our opportunity to grow in the future. Managing a team of software developers over time with varying levels of experience can be a hard thing to do well, and I welcome tools that help give me insight without having to micro-manage to the code review level.
As a guiding principle for an organization, investing wisely in how developer time is spent will go a long way to avoiding problems of compounding technical debt, and maintaining low technical debt will help keep velocity high, outages low, and help keep developer confidence in their ability to do the right thing high.
There’s a lot more to static analysis, both in terms of the broader range of metrics available and the other aspects of static analysis as they relate to best practices, even down to how code is formatted.
This article originally appeared on Medium.