Testable code is cleaner code

I have just spent about 9 hours of my weekend spread across two sessions providing after-the-fact unit tests for about 30 methods of varying sizes (10-20 lines) written by a very much junior colleague, with the goal of getting complete line coverage, save on an irreducible minimum of calls to the outside world (in the form of external libraries being consumed by the project in question, in the main).

The main reason why the code wasn't being unit tested at the start of the exercise was that the methods were usually structured as, in the most abstract form

which meant that tests were pretty much snookered from the outset, with the code as it stood.

So, refactor

Which has removed a level of nesting conditionals around the main business, and created private methods that maintain the object's abstraction -- nothing has been made spuriously directly visible to the outside as internal or protected. If these methods were public, they would be directly testable; as it is, within the test environment they can be tested by reflection, by just feeding in the appropriate result.

And the code is cleaner already -- the specific concern of each method is now clear, and the important part of business, as extracted into ReactToSuccessInTheOutsideWorld can be tested and refactored to tighten up the first-draft flabbiness. This will usually be along the lines I outlined a couple of years ago. The retrospective tests indicate the corners that cannot easily be reached, or require a lot of duplicate set-up, and point to refactorings that keep all previous tests passing, while yielding better coverage with fewer further tests -- which should mean more cleaner code.

One of the things along this line that was brought to light by this stage of the exercise was a C# peculiarity which is at least explainable in hindsight:

where that WTF is indeed considered reachable code by the compiler, even if unreachable in practice and a blot on the test coverage -- whether because the equality and comparisons are overridable operations which could in general be wrongly implemented, or because mutable member references could potentially be changed between comparisons by a concurrent thread, it does not matter. Where we are sure that the instance is thread local, and the comparisons are sane, we can lose code

Of course within the time so far expended, the refactoring hasn't gone quite as far as pulling all those bits indicated here as separate routines out from being in-line, and many of the tests are still horribly cut-and-paste. But I can now refactor both the product and the test code, knowing that if tests fail or coverage drops, I've broken something; and I can also feel that the code under test is in a slightly better state than I found it -- both somewhat more partitioned, and with a couple of "that test didn't ought be failing like that" actual bugs fixed.