Today’s post marks a bit of a leap into the unknown for me, as I explore using static analysis to improve my code with NDepend. I explain how it can be hard to know where to start, but also how valuable the insights can be. The actionable changes are demonstrated with commits from my development of ConTabs. Finally – in keeping with the expositionary style of this post – I close with some general observations (including an attempt to explain the difference between NDepend and ReSharper).
Static analysis used to seem to me like an exotic (and expensive) luxury; the preserve of consultancies and massive enterprises. The term makes it seem so complicated. Two things have changed my mind on this. The first has been reading Erik Dietrich’s blog, where the “static analysis” tag is frequently used. The second factor has been conversations with a colleague, where we really dug into the meaning of the term.
The truth is that “static analysis” simply refers to any reasoning applied to code without running it. If you’ve ever read through some code (perhaps during a review) and considered what it would do, you’ve performed static analysis. Of the other tools I use day-to-day, both Visual Studio and ReSharper perform static analysis to a lesser or greater degree respectively: Visual Studio’s IntelliSense exhibits a limited form, whilst ReSharper uses static analysis to make significant refactoring suggestions.
First impressions of NDepend
When Patrick Smacchia got in touch to suggest I tried NDepend, I have to admit I was a bit overwhelmed. NDepend is a powerful tool that, if I’m completely honest, I didn’t feel qualified to use. I mean, isn’t NDepend what real programmers use? People like Erik Dietrich and Scott Hanselman evangelise about it.
When I fired it up for the first time, I pointed it at a large project at work. The report was, well, OK.
I seem to recall the overall grade was a “B” and there were lots of numbers on the screen. My first thought was “now what?” I saved the report and sent it round to the team, who mostly responded with some variant of “huh, neat.” None of us had used such a powerful, comprehensive tool before and we didn’t know how to dig into it.
Over the next few weeks, I would continually open up the NDepend dashboard and gaze at it, clicking around the different widgets, reports and visualisations. It seemed impenetrable. I made a few changes to the code based on the built-in rules and “quality gates”, but never had the lightbulb moment of wisdom that I think I’d been subconsciously expecting.
NDepend and ConTabs
Not getting anywhere with it at work, I decided to take NDepend home with me and run it on ConTabs. This is when it all started slotting into place. I’m not sure whether it’s the fact that the ConTabs codebase is so much smaller, or whether it’s the fact that I know it so much more intimately, or perhaps because there’s no legacy code to worry about – whatever the reason, it finally clicked. (And it wasn’t really a lightbulb moment, but more of a gentle dawning.)
I ran the first analysis early in development and then ran it again and again. I started to learn some really interesting things about my code. For instance (as I write this) ConTabs has 176 lines, 7 types, and 63 methods. Comments make up 7.85% of the codebase. The most complex method is
StringValForCol, with a cyclometric complexity of 7. (I could go on – NDepend has metrics for anything you can think of.) Particularly interesting is the way NDepend displays metrics alongside their “diff” since a (movable) baseline. I can tell, for instance, that between this build and the last build I’ve mitigated 7 rule violations.
Soon enough, I started to make some changes to address some of the built-in rules. First up was something I already knew was a bit janky: the
Style constructor with 11 parameters (one for each possible edge piece). This triggered the “Avoid methods with too many parameters” rule. This is a medium severity violation and came with an estimated 1 hour and 36 minutes of technical debt.
I fixed this issue by creating a
Corners class and using it to encapsulate the 2D array of
char. This only half solved this issue, so I also moved to an object initializer syntax. (I wrestled with this, as the constructor feels like the right place to get this information – what does it mean if these
chars aren’t specified? – but that constructor was a real pain to work with…) These changes are in commit a11d0e37.
Quickly getting into the swing of things, I then nested the
OutputBuilder class inside
Table (79b98469) and could then fix its access modifiers too (357fae50). I also made a backing field readonly (9f722d5a).
Then I came across a problem… When I’d refactored my Style class by offloading some of the work into the new Corners class, I’d changed the public API. Several methods and a field were no longer available. At the time, this was semi-conscious, but I perhaps didn’t give enough thought to the people who may already be using this functionality.
Luckily, because of the “diff” functionality, NDepend has rules not just for what the code is now, but how the code has changed. In this case, I’d triggered “API Breaking Changes: Methods” and “API Breaking Changes: Fields”. Appropriately scolded, I duly reintroduced all of the methods and marked them as deprecated using
Gold stars all round, right? Not quite…
Custom rules with CQLinq
Having these methods marked as obsolete did satisfy the NDepend rule about breaking changes to the API. One of these methods was the “too many parameters” constructor mentioned above. The problem was that the “too many parameters” rule didn’t give a damn that the offending method was now marked as obsolete – it still had too many parameters!
It was at this point that the value of one of NDepend’s core philosophical underpinnings became abundantly clear to me. All the rules (even the built-in ones) are themselves just code. They use CQLinq, which is Linq for performing Code Queries. This means that rules can be tweaked as needed, or even written from scratch.
It was trivial for me to modify the code that makes up the “too many parameters” rule to ignore methods marked as obsolete:
warnif count > 0 from m in JustMyCode.Methods where
m.NbParameters >= 7
orderby m.NbParameters descending
The “Queries and Rules Edit” pane positively encourages you to switch into the source code of the rules. CQLinq itself is so natural to read that dipping in and making changes is almost irresistible.
Other miscellaneous observations
I’m still early in my NDepend journey, but this is already a long post, so I’ll wrap it up here with some final thoughts.
If you plan on using NDepend, make sure you’ve got two screens. The information density makes this a complete necessity. Working on my laptop on the sofa at home felt very claustrophobic.
If you’re used to ReSharper, prepare for a bit of a shock to the system. Where ReSharper focuses on providing solutions in the form of instant, push-button refactoring, NDepend offers much less hand-holding: rather than automatically rewriting code, NDepend identifies problems and provides guidance on their resolution. The trade-off is that the rich data provided by NDepend empowers the skilled developer to make design decisions that cannot be automated. As a result of not being constrained to what it can fix, the suggestions NDepend can make are much more wide-ranging that it would be reasonable to expect of ReSharper.
I’ve not touched upon the ability of NDepend to analyse test coverage data. This is going to be tricky for me, as I use an open source coverage tool (OpenCover) whose output is not natively supported. Nevertheless, I’m going to have a stab at this and plan to make it the subject of a future blog post.
One interesting side-note in relation to code coverage (rather than static analysis), is that Coveralls reported a big hit when I first pushed a commit including all the methods I’d marked as obsolete. This made some sense. I mean, what’s the point in testing code already marked as obsolete? Especially when the code replacing it is fully tested. The workaround here was to modify the OpenCover command in the AppVeyor configuration to tell it to exclude code marked with the “Obsolete” attribute.
In this post, I’ve tried to give as succinct an account as possible of a fairly rambling, disjointed discovery process. I’ve felt at times like Bambi on the ice. It was tempting at times to try to sanitise the experience and make myself seem more expert.
The reason I kept it authentically “rustic” was to reassure other developers: finding NDepend bewildering doesn’t make you a bad developer. I hope the valuable insights I was able to glean for the ConTabs project will encourage other developers like me to work through the bewilderment – it really is worth it!
I’ll be revisiting NDepend in future posts, so stay tuned. In the meantime, if you’ve got any suggestions on how I should be using NDepend, please share them in the comments below. (I’d be over the moon if you knew a good way to import OpenCover data!)