As you could read in the post about NDepend – the Technical debt feature was I think the best what was new (to me) in NDepend 2017. I’ve decided to use it and write couple of posts how I use this feature to actually reducing it in the NetDevelopersPoland WebSite project.
Our baseline is not that bad. It’s reporting only ~12% of technical debt – it should not be hard to get rid of it.
Lets see what classes are the main contributors to this value. We click the 5h 12min effort to reach B to drill down.
If we check the Issues for the first module (FacebookModule) we see that the issues that we need to deal with are:
- Avoid prefixing type name with parent namespace name
- Constructor should not call a virtual method
- Potentially Dead Methods
but by quickly looking at FacebookModule class we can only identify the first issue. How come the other two came to existence?
Let’s look at the “Constructor should not call a virtual method”. Of course we don’t have to look far away from NDepend as it already has all the info we need. Just click on 1 method and you get all the info neede. Our faulty method is Request. But one might ask why we shouldn’t call virtual methods in ctors?
The issue is with non-sealed classes. If someone inherits from the class and overrides the method, when it is being called the derived class might not be yet initialized.
So here the fix is to sealed the class as we do not expect anyone to derive from it. And after that our technical debt has lowered. We also identified couple more Modules that have the same issue – fixing them will give us another boost in lowering the debt.
Method? What method?
What about the third issue – Potentially Dead Methods? From the first look at the code we are sure that they are used, but probably NDepend doesn’t recognize that our methods are called from within the lambda. Lets see if we can do something about it.
First let’s deal with GetPostsWithTag. It’s simple if we look into the decompiled code.
If we check our code in dnSpy we noticed it is in fact something else!
Due to the fact that we deal with dynamic call to our method is done via CallSite. We could try to change the rule that generates those warnings but it looks like the model lacks some information in here.
as this check is I think not enough & not fast enough to be kept. It would be best to be able to check if the parameter to InvokeMemeber is equal to our method name. If this is not the way – maybe we can get rid of them at all?
GetPostsWithTag can be quickly solved by removing dynamic from the method call
GetValueFromDynamicOrDefault is used to extract correct values or defaults from route. If we could do that outside the method we would get rid of it.
And it looks like we can. Nancy has the ability to use Custom segment constraints. If we would write those as those constraints we could get rid of the method from the Module.
And use them like this
With the month parameter we can do the same but if we want to use Custom segment constraints with options parameters. We need to to something different.
We splitted the handlers. And I think it’s even better as each handlers is dealing with a specific case. One that deals with full year and one with only a month. Next entry will deal with rest of the issues. This is new baseline.