Here are a few thoughts about the refactoring process from going through my initial unit testing iteration.
- Constructor Injection can make things look a lot more testable. But I think you want to be careful about jumping into this – don’t do it as the first step in refactoring. It’s probably better to first extracting bits of code that don’t have strong dependencies – and don’t need to know about a class state. Creating some beefy static methods that do a lot of work if you give them a few parameters, even if you could have gotten them from ‘this’ if you did a non-static method, can just make life easier unit testing.
- Once you’ve extracted some dependency-lite code, what you should have left over is some dependency-heavier code where DI is a better, naturaler fit.
- Sometimes what you are injecting is not a ‘runtime dependency’ it’s just a piece of policy or logic or configuration that doesn’t feel like something you want to hardcode in this particular class, or that not hardcoding can simplify the testing of. Think of a mapping of types to ID numbers or something that feels so arbitrary. Unfortunately DI isn’t a natural fit for this because DI works in terms of types.
- Separating enumeration/grouping control flow logic from actual data processing can make code look really nice, where classes/methods have clearer responsibilities, and make tests simpler too. It can also lead to having more classes.
None of that this refactoring should change your block count much. And it takes time, and then you still have to write the unit tests, so it feels frustrating.
After my initial bout of refactoring I realized this was very slow going, and started looking for cheaper ways to get coverage up. Oh wait, I already have a folder of known dead code? Delete! That feels good, but wasn’t as big a % boost as I was hoping though.
After going down this road a bit, you get to a point where you think – I won’t need dedicated complicated factory classes if I switch over to using a Dependency Injection framework. This brings up another worry though. While you no longer need code coverage of a factory class, do you need to replace it with code coverage of your dependency container setup? Is there really no getting away from stupid looking tests?!
When I switch over to DI, I often see a couple nice code simplifications fall out of it. But, still coverage gets slightly worse overall by the refactoring – however, it’s now crystal clear that 55 of my 599 uncovered blocks are dependency injection container setup/binding/convenience/wrapping. For some reason that feels almost as good as actually covering that code. I think the reason is that I know this code runs every single time I run my app, so it’s going to get a lot of real-world coverage as opposed to unit test coverage. And I can feel that confidence in a way that I just don’t feel for an arbitrary factory class that is floating around.
By this point the biggest class I have with zero coverage is my DatabaseContext, which has a whole pile of helper methods on it. This brings me to the thorny question of how to unit test EF database queries… which is probably going to be an article in itself, so I’ll come back to that another day.
Aside from that there are two largish classes with lots of logic I can try to figure out how to test, that now have much nicer factored dependencies.
While I’m not sure if it’s a valid topic for discussion I’ve also noticed a couple random small things you can do to reduce your overall # of code blocks along the way.
- Explicit default constructors increase your block count.
- Async/await increases your block count. And rarely leads to subtle bugs. Why use it if you don’t actually need it?
- Assertion helper classes and other ‘correctness helpers’ can have logic inside them which does annoying things to your coverage numbers if part of the project you are measuring – unless you either exclude them from coverage, which you can do with ExcludeFromCodeCoverageAttribute, or move them out to another project.
The idea of creating either a MyProduct.ArgumentValidation library or a MyProduct.Comon library really doesn’t sound good.
Everyone knows this pattern of saying Require.NotNull(foo, “foo”), or Argument.Required(foo, “foo”) right? Which means you only have a single line of code [to test code coverage of] instead of a zillion if statements, in each of your methods?
But just how many times does this particular wheel need to be reinvented?
Well it turns out, you might finally be able to stop and never write that class again. Someone called ashmind made a MIT-licensed open-sourced-on-github nuget package for that.
Jolly good, I say! Now, if our lawyers happen to think adopting this package this is as good an idea as I do, I should be able to avoid writing unit tests of another 12 blocks.
So honestly, I don’t know if I’ll get to use it. But I feel this package deserves some promotion. It is an earnest attempt to scratch out an itch, so I want to say try it out, and if you see anything it’s not good at, you can help make it better!
[G’day ashmind – you didn’t ask for the publicity – I hope you don’t mind.]