2011-06-22

Big Refactoring: when two became one a.k.a. why do I need tests and test environments

We had a issue these days very common to the dev world: some of are models didn't represent very well our business. Wrong naming and wrong modeling always gets me crazy. This time we had two models that didn't represent very well as different ones and we thought it was better to merge them in a new model better named. The real challenge is that they were some core models in our system, so there was a lot of changes involved.


So what is the best way to refactor this code?
- We could make lot's of small changes, transferring data gradually from one model to another (less risky and with less impact, taking a lot of time)
- We could make one big change at once, transferring all data from both models to a complete new one (more risky and wit more impact, taking considerably less time)

We went for the second choice, but with a slight difference as we killed only one model, instead of both. Leaving the creation of the new model to a second step.
Since we had lots of tests in our system and a complete test environment, the risks and impact of a big refactoring are dramatically reduced. Then, it took only a couple of hours to create db migration, merge all logic in only one model and use sed to replace its occurrences inside the code.

Some details here, that will help your refactoring be easily rolled back:
- when refactoring models that have relationship with other models, keep the old column in the database
- in case of killing some models, keep the old tables instead of deleting them
- always have a rollback plan
- make your deploy/rollback tasks as automated as possible

Well, it's done. Let's run the tests. 59% of failure. We forgot to made changes in the test factories to represent the new model. Changes made, let's try again... 100% of success.

The next step is to write a rake task to migrate all data into only one model. Why not doing this inside the migration files? 2 reasons:
1) You should never use the model objects inside them, because, as in this case, you may need to rename them and your migrations will break
2) I'm not that good writing SQL to use the execute method for populating the table and fixing relations between the models

Once the rake task is done, we deployed this code in the test environment and did some smoke tests.
All going fine, we felt more confident to use this in production. \o/

Now we just need to deploy in our production environment.
A core refactoring with not much pain, thanks to tests and a good test environment for running smoke tests.

No comments: