Skip to main content
13 of 14
wording improved
Doc Brown
  • 220.6k
  • 35
  • 410
  • 625

It seems you are refactoring "just in case", without knowing exactly which parts of the codebase in detail will be changed when the new feature development will take place. Otherwise, you would know if there is a real need to refactor the brittle modules at stake, or if you can leave them as they are.

In my experience, that is a doomed refactoring strategy. You are investing time and money of your company for something where noone knows if it will really return a benefit, and you are on the edge of making things worse by introducing bugs into working code.

Here is a better strategy: use your time to

  • add automatic tests (probably not unit tests, but integration tests) to the modules at risk. Especially those brittle modules you mentioned will need a full test suite before you change anything in there.

  • refactor only the bits you need to bring the tests in place. Try to minimize any of the necessary changes. The only exception is when your tests reveal bugs - then fix them immediately (and refactor to the degree you need to do so safely).

  • teach your colleagues the "boy scout principle" (AKA "opportunistic refactoring"), so when the team starts to add new features (or to fix bugs), they should improve and refactor exactly the parts of the code base they need to change, not less, not more.

  • get a copy of Feather's book "Working effectively with legacy code" for the team.

So when the time comes when you know for sure you need to change and refactor the brittle modules (either because of the new feature development, or because the tests you added in step 1 reveal some bugs), then you and your team are ready to refactor the modules, and more or less safely ignore those warning comments.

As a reply to some comments: to be fair, if one suspects a module in an existing product to be the cause of problems on a regular basis, especially a module which is marked as "don't touch", I agree with all of you. It should be reviewed, debugged and most probably refactored in that process (with the support of the tests I mentioned, and not necessarily in that order). Bugs are a strong justification for change, often a stronger one than new features. However, this is a case-by-case decision. One has to check very carefully if it is really worth the hassle to change something in a module which was marked as "don't touch".

Doc Brown
  • 220.6k
  • 35
  • 410
  • 625