Lesson learned from a very hard project
It’s been a while since my last post. Well, I was really busy with my project last year and I have been on a deserved 6 month sabbatical since June and I came back to work at hotels.com one month ago.
One of the things I wanted to write about before leaving London 7 months ago was to write about the lesson learned on my last project: Communication is – by far – the most important thing in a team. Yes, many people say this. You think you know it. I thought I knew it. No, you do not – and I did not – know it enough.
The project was about migrating the view templates written in JSP to an in-house technology based on Google Closure that we called Dionysus internally. The challenge was to move important highly coupled untested legacy code to a new platform keeping the same behaviour and look & feel so the user could not notice. It is important to mention that any error in this module could have considerably impacted revenues
My idea was to create a test harness based on acceptance tests to reflect the multiple rules we have in the site and that they were bad-maintained in the form of out-of-sync wiki pages. The second – and most important – thing was to use unit tests to allow us to detect bugs quicker and to refactor code with confidence. My big problem here was the legacy code:
- Abuse of the visitor pattern. It was all over the code.
- Abuse of spring configuration files. In other words, an important part of the app flow was defined in xml spring context files.
- Obviously, no tests at all.
- Lots of problems that made unit testing almost impossible to most of the classes: no separation of control-logic-data (3 tier architecture), static methods everywhere, methods and classes doing lots of unexpected things, etc. See Misko’s guide to write testable code.
- Lack of infrastructure to do continuous integration properly.
I could not see a way of doing this without rewriting an important amount of logic, putting every single thing in its place. In order to do this we (the team) first agreed on:
- Creating user stories to every single feature on the site
- Applying the 3 tier architecture (by applying Domain Driven Domain concepts) as much as possible by moving logic to the write layer: view flow in the controllers, rules in the domain layer and data access in the repository layer.
- Write tests for all business scenarios (acceptance tests) and technical scenarios (integration and unit tests)
The first lines of code
Once we started to code the first stories I found my very first and big problem: everybody was coding in the old style, I mean, reusing the same crappy classes and adding patches to them. No separation of concerns (3 tiers), static methods again, etc. There was an attempt to write tests but they were being abandoned as testing the same crappy classes was very difficult. Developers ended up creating very long test methods full of mocks. The day I decided to do something was when someone suggested to start using PowerMock. Seriously, I hate that tool. If you need it, you are missing the point of unit testing.
In order to fix this first problem we decided to code review every commit. This brought the second problem: big commits due to big stories. How a human being was going to be able to review properly a one or two weeks of work? and how was I going to force a colleague to throw away one or two weeks of work and redo it again? Also, this brought lots of waste of time and discussions as everybody thought they were right about their solution.
I soon came to this conclusions:
- We did not have a common criteria to do code reviews so people was applying their own (right or wrong) criteria which lead us to arguments, waste of time and abandoning the idea of doing code reviews as nobody wanted to have more arguments or to more waste time.
- If we abandoned code reviews, we would be doing things in the same way as they were done before.
- If the things were being done as they were before we would still be having bugs and the code would be difficult to maintain.
- If code was difficult to maintain, I would have to work more hours in a very unhappy environment.
Time to stop, think and talk: communication
One day, after many days of frustration, I suggested to stop and talk about the problems I detected. We had a hard week of discussions but we agreed on the following:
- Follow the same coding criteria by using the 3 tier architecture and DDD concepts: context boundaries, real domain objects, etc.
- Respect code reviews
- Commit more often to allow smaller code reviews
- Stop and discuss any doubt.
- Talk talk and talk over code code and code
The following months
We agreed on following those rules. In the beginning we did lots of pair programming to spread the knowledge. This allow developers to understand key DDD concepts and how to write nice unit tests. In the beginning, we spent more time spreading knowledge than coding but this paid off in the long term as in the end we were 6 developers coding with the same mindset very quick. Code reviews were done quicker, we did not have many bugs and code was reused easier.
Five months later, we finished the project in time, we rolled out the app to production smoothly, without big issues; now we have few bugs (one every 1-2 weeks as opposed to 1-2 every day) and it is 45% faster then before.
Another extra benefit is that the second phase of this project is being done extremely fast because we all think as one and we are using most of the code done in the previous project.
In a third phase, we will get rid of bits of complex code that we could not rewrite but it was isolated, wrapped in a repository and translated to our domain language.
Communication is the fastest way of coding as a team.
Communicate, communicate and communicate.