Lessons from refactoring #1

We are in the process of moving a monolithic application, into a modular one. So we split it horizontally into layers and vertically into domain specific functionalities. The lowermost  layer, as most people call it as data access layer, was responsible for fetching the data from external systems. The layer above it serves as a translation layer that converts the data from external systems to a consumable format (shedding the unwanted info, converting the errors to exceptions, etc) and also do some processing on it(which people generally call as business logic). The User Interface/Presentation layers sit above these. As usual, we didn't have all the time in the world so we kept the code from the monolith the same for the lower layer and wrote the business layer from scratch.After all, its the business layer to which the applications will depend on.

I still think, the thinking was right. But where we missed was how we validate the business layer. It was designed with unit testing in mind.It was beautifully written with interfaces and provisioning for dependency injection and voila,we were proud that we had a loosely coupled code. No No No,, that is not the bad thing, but that loosely coupled code did not do the right thing, that's BAD. That module had a unit test coverage of  99% and still the unit tests did not identify that our beautiful code is not doing the thing that it is supposed to do and that is VERY BAD. 

So where did we go wrong? 

1. We referred(isn't that a nice way of saying copied) the old monolith code base to implement the new modules.
• Despite the fact that we really started referring, very soon the developers started copying the code out of the old application. The monolith was started almost 6 or 7 years ago and since that day, the code was always added and never removed. Which means there was a ton of unused code in there. All these code got into the new module with nice names and a good way of using them. Except that they were never to be used.
2. The unit tests were written to test what was coded.
• As I said earlier, half of what we wrote was crap and we had unit tests that tested almost 100% of that crap.
3.  We believed since unit testing is in place with a 100% coverage, we can relax on reviews and churn more code and tests. Working code over anything else(We're agile!!!).
• None of us reviewed the quality of the unit tests.

So the lessons learnt:

1. We should have started with the real requirements in place. We should have never said, "Guys, We need to make this monolith modular, believe me or not, you got a chance to write a module from scratch". What we should have told is, "Guys, Here is what that needs to happen, write a code that does it". If we have done that, all that dead code that was taken in would have been avoided.
2. We should also have said, "Wait a min guys, before you go and crank some code, can you write some unit tests around "Here is what that needs to happen". So if even if break my neck today those tests will exactly say what I wanted you to do". Yeah, we could have done Test Driven Development, not exactly in the way uncle Bob says((forgive me uncle Bob), but at least there would have been tests that validate the code for the expected behavior rather than for what is already written.
3. If we would have done a review of the unit tests, we would have found out that the unit tests were just bogus.

But its not that late...#3, happened and the result is this post. Me and my team are now on to do #1 and #2. See you soon.

Ironically, I got to see this post today :)

http://blog.ploeh.dk/2015/11/16/code-coverage-is-a-useless-target-measure/

I am with you Mr Mark Seemann