My friend Dom Finn wrote a post To Fix Or Not To Fix. This is something I’ve thought about quite a lot as, lets be honest, everyone is maintaining a legacy codebase to some degree.
The Boy Scout (or Girl Guide) Rule is to leave the campsite tidier than you found it. This helps avoid making the problem any worse and the propagation of Broken Windows.
What motivates refactoring?
The question Dom raised is how much is too much and when should you refactor? Firstly, this is being done as part of a change with a business motivation isn’t it? If you are refactoring for the sake of refactoring, without some wider goal, you are wasting time that could be spent adding value. Refactoring should be in service of a change, not the change itself.
You revisit important sections of code frequently. If all you do is make a small improvement each time you alter a section of code, if that really is an important area of the code you’ll be back soon enough and those little improvements will compound over time. Also, each time you revisit the code you’ll know a bit more about the domain and be able to refactor more intelligently. Today’s obvious grand refactor could be tomorrow’s retarded idea. Delay major decisions as long as possible.
Everything you didn’t write is shit
The motivation for refactoring code comes from the fact you think it is shit. For some measure of shit. It might not be using the right pattern for your liking, it might be formatted wrong, it could be written in the wrong language.
That could could be yours that wrote a few months ago leaving a lot to be desired. There are many causes of this, you’ve improved as a programmer in that time, or you may understand the domain better with the experience gained.
As for other people’s code I always try, though it can be hard, to assume they had some deeper insight than I have right now, or at worst they just didn’t know any better than to create the travesty in front of me. The first thing to remember is that people generally do the best they can under whatever pressures they were under at the time.
Gain objectivity over the problem by removing the emotion from the situation. If you’re angry and start a code deleting frenzy you will cause more problems than you solve. Refactoring is a time for a cool, methodical head. Particularly if that code has little or no tests and you don’t fully understand what it is doing. Dump your emotions before you contemplate starting.
How much should you refactor?
As much as you need to get the job done in a sensible manner. Any refactoring you do needs to be kept within the context of the task that led you to that code. Don’t blow your allocated time because you’ve spent 3 hours of a 1 hour task tidying up because the code was “such a mess” without having made the change yet.
For me, there is an initial stage of refactoring where you move the code around, making it abide by coding standard so you can read it, commenting methods as you find out what they do. All this is to help you understand the code and identify where you need to make the change. This shouldn’t be more than 10% of the overall time allocated to the task.
Once you’ve identified where the change is required, consider reverting some or all of the changes you made in order to get there. Have you made the code better or just moved it around? If you’ve just moved it around perhaps revert it, it will make the intent of your real change clearer in the history and you will have changed less, reducing the possibility of introducing new bugs.
At this point you may think that your change will be easier to make if only you refactor this area first. This may be right, but always be sure to keep in mind the scope of the original task. If you have one day to do a task and refactoring for half a day will make it a 5 minute change that could be acceptable. But always keep in mind the risk you are adding by altering what is, I hope, code that works.
Don’t be afraid to throw changes away. Try that refactor and if you’re 30 minutes in and the rabbit hole just seems to keep getting deeper, stop. Get your bearings. Consider that going back to where you were and doing it without the major refactor could be the more pragmatic approach.
All this is made much easier if you commit often, with explanatory commit messages. Being able to step back 30 minutes to after the general tidy but before attempting the grand refactor gives you an easy way to back out.
I’m not you, I don’t know your code base
There are no hard and fast rules, every situation is different and you have to use your own judgement to decide the best course of action. All of this is just how I approach the problem, the steps I go through to try and avoid wasting time making changes that don’t really need to be made.
Every developer loves a good refactorbation session but we’re not paid to pleasure ourselves, we’re paid to use our skills to make our companies money.
Keep that in mind at all times and good luck out there.