Don't refactor code unless you need to change the code for a business reason.
One of the common mistakes I often see with regards to refactoring is when people refactor code that doesn't need it under the guise of making it better. The argument usually goes something like "this needs to be more abstract", "I wrote this code a long time ago and it is crappy", "this code is too complex" or something along those lines.
You should only refactor code when you are already in the code to make a change to support the business. That may sound counter intuitive but one of the worst things we can do is change code, however crappy, unreadable or complex that doesn't have a reason to change.
Valid business reasons to change code include (but are not limited to):
- Adding new functionality
- Extending existing functionality.
- Making measurable performance improvements.
- Adding a layer of abstraction in order to support a new use case.
- Modularizing a particular object so that it can be reused in another part of the system
Adding new functionality or Extending existing functionality
This is where the boyscout rule comes into play. If you are in already in the code for another reason then you should clean up the code even if you didn't make the mess.
Making measurable performance improvements
This one is probably self explanatory but it's important to note that performance improvements will usually require some level of refactoring.
Adding a layer of abstraction in order to support a new use case
This is an important one to understand. Often people will over generalize code at the beginning. This leads to overly complex designs and less readable code. If we follow the rule of not creating a layer of abstraction until we have at least two or three use cases for the code then there will come a point when you need to refactor the code in order to provide a layer of abstraction that doesn't already exist.
Until that second or third use case comes about the code should not be generalized. You don't have enough information about future uses of the code to get the abstraction correct. You may get lucky and guess at the future abstraction but you don't want to run your business on guesses and luck.
Modularizing a particular object so that it can be reused in another part of the system
Code reuse is one of the most important tenets of object oriented programming. When we identify code that is not specific to a particular object or package AND is needed in some other part of the system we should refactor this code into it's own module. Its important to ONLY do this when the code is actually needed in another part of the system.
Don't refactor code without tests
In order to refactor code safely you should have unit and integration tests for the existing functionality. I would also argue that you should write tests for the new functionality as well before you refactor. This will help you to understand the proper way to refactor the code as it helps you define how the refactored code should be used from a consumers standpoint.
If the tests don't exist for the the existing functionality you should write them first before you start refactoring. This helps ensure that you don't cause a new bug in the code or regress an old bug when refactoring.