Refactoring is part of a software developers work. General rule of thumb I always used is if you see code that smells (for instance duplication) you should refactor. But does this rule always apply? Are there any situations when you should not refactor? And if so, how do you recognize those situations?
Last week I worked on a particular task, let's call it task X, where I needed to use a service with an unclear interface and a smelly implementation. After consulting with the rest of the team we decided to refactor this particular method of the service so it will be more clear and remove code duplication in that method. But while I was refactoring I noticed the other methods in the service were also unclear and contained code duplication, so I started to refactor those as well. To proof the service still worked as before I needed a good (automated) regression testset. It was production software and I wanted to make sure I didn't break anything with my refactoring. Unfortunately there wasn't an automated regression testset for all the methods in the service. So while I was on the verge of creating a complete automated regression testset for the entire service that, amongst other things, would have consisted of mocking a webservice call I remembered something about shaving yaks.
Why would I create an automated testset for the entire service, while I am actually working on task X? So before stuff get out of hand I decided not to refactor the other methods in the service, leave the code duplication in place and just stick to refactoring (and testing) that one method in the service.
What made me decide not to refactor?
- The refacoring did not have a direct connection with task X I was working on
- There was no automated regression testset available
- The rest of team agreed with not refactoring
- The complete refactoring (incl testing) would have taken more than a single day to finish (at least a week)
- A good practice in agile modeling is only update when it hurts. I think this also applies when it comes to refactoring code. I wasn't using the other methods of the service and therefor it did not hurt me
With pain in my heart I had to leave the smelly code alone and went on with implementing Task X. Does this mean the service stays smelly? I think that will depend on the future functionality we are going to implement and if we need that same service again. And if we do we will probably apply the same rules as we did in this situation.