This is the second blog of a series of undetermined length about things NDepend told me about a personal project of mine. The first post describing the project (an object-object mapper) and the first change I made is here.
So last time NDepend told me about an abstract class I had which had an internal constructor - cool :) I next looked at a claimed 40 methods which are never called - this is the screen opened from the Dashboard describing the methods:
It’s the same set up as last time - the Linq query and an explanation of the rule are at the top,
the results are at the bottom. After clearing out a few methods which sure enough were never called,
I got to the Clear()
method:
…which the report says is called by two other methods. So why is it included? I turned to ReSharper to find where it’s called:
…and yep - it’s called in two places. Let’s look at the NDepend Linq query to see if there’s a clue
there as to why Clear()
has been included:
Ahh, ok - so Dead Methods are not only defined as uncalled methods - they include methods only called by uncalled methods! That’s rather clever :)
This is where the way I set up the report might have bitten me a bit. I have a project for the mapper
and a project for the tests, but I didn’t include the test project in the analysis because I’m most
interested in the mapper project. To avoid having to work out what and how to map all the time, the
mapper does quite a lot of caching - but I want my tests to be isolated, so that’s where methods to
clear the Cache
come in. In other words the uncalled methods which call Cache.Clear()
are
called, but only by the test project, which I didn’t tell NDepend about. So nothing to delete there.
Ok, next one:
GetDeferredAssignmentTargets
. This screenshot also shows NDepend’s pop-up info box. Here’s the method:
Ahh, ok - I’ve already told ReSharper not to worry about this one - it’s called via reflection. This is a perfectly understandable shortcoming of both ReSharper and NDepend, so nothing to delete here either - the Linq query will actually ignore methods marked with a particular attribute, so I can exclude this one from the analysis. Next one:
Apparently the constructor of MappingStepActivityDescription
is never called. Well, it turned out
that class actually didn’t have a constructor, but all its methods were static, so I was able to make
the class static too:
Nice! :)
All the methods in the report fell into those categories - never called, only called from the test project, only called via reflection, or classes which could be made static. I got to trim out a reasonable amount of unused code, with all my tests passing to let me know I hadn’t broken anything. Good stuff!
I next looked at the next Critical Rule violation, which was an Exception not named with ‘Exception’ on the end. I’ll write about that next time.
Comments
2 comments