Is it the correct practice to keep more than 10 years old spaghetti legacy code untouched without refactoring at all in big product development?

Question

I have been in two software product houses for three years in a row.

The first is a small company maintaining a fairly small management system with a monolithic legacy code base (almost twenty years). Tightly coupled code is everywhere without sufficient unit test coverage. However, the management usually does not want developers to refactor the legacy code.

The second is a fairly big company maintaining a big domain-specific system with a huge monolithic Java legacy code base (over ten years). The layered architecture indeed decoupled the infrastructure from the business logic. However, in their business layer, there are also some giant classes with more than 3 thousand lines of code. Developers still continuously inject more and more code into those legacy classes. Developers are allowed to refactor their own fairly new code about adding new features, but are warned not to refactor these giant spaghetti classes, either. Experienced senior developers say that changes or refactoring on those classes might be disastrous due to the lack of regression tests.

However, personally I have read practical books about clean code and refactoring. Most of the books strongly recommend developers to refactor actively. But why in real world companies are against this?

So I would like to collect answers from very experienced developers. Why do these two companies I was in prefer to keep the super legacy code unrefactored? Isn't this disastrous?

Answer 1

It‘s a question of risk management:

• Refactoring a system always creates the risk of breaking something that worked before.

• The larger the system, the higher its complexity, and the higher the risk of breaking something.

• With spaghetti-code (or any other poorly structured code) the real structure of the code remains fuzzy, and the dependencies might be hidden. Any change in one place could easily have impacts anywhere else. This increases the risks of breaking something to the highest level.

• With TDD, or any other technique guaranteeing a comprehensive set of test cases, you can quickly verify that refactored parts (and dependent parts) still work. Of course, this is effective only with the help of proper encapsulation.

• Unfortunately, tests are often missing for legacy code, or their coverage or depth is insufficient.

In other words, with large legacy spaghetti code bases, refactoring creates a high risk of breaking something that worked before, and the impact of this risk cannot be reduced with automated tests. The significant risk of refactoring simply outweighs refactoring benefits in this case.

Additional remark: An alternative approach at a lower risk is: don't touch the running system, but implement new and replaced features with state of the art testable code and clear boundaries. This more evolutionary approach is not always feasible, but it can provide significant short term and long term benefits.

Answer 2

One reason is it's really difficult to measure the loss of productivity the messy code is causing, and difficult to estimate the work it will take to clean it properly and fix any regressions.

The other reason is many developers incorrectly call any rewriting refactoring. True refactoring is very methodical, almost mechanical. You make very small changes that improve the structure but don't change the behavior, and you verify the behavior didn't change using automated tests. If there aren't good tests, you add them first.

It makes experienced developers nervous when someone says they don't understand some code so they need to rewrite it. Changing code you don't understand is a recipe for disaster unless you are making dead simple changes.

So if you want to refactor some code, make your change so dead simple that anyone reviewing the pull request can instantly see the behavior is preserved. My first pull request in really messy legacy code is usually nothing but tests. My second pull request is purely mechanical changes like extracting repeated code into a function, and more tests that take advantage of those changes. Then on the third pull request I might start to rewrite some (now much smaller) functions for clarity, now that I have thorough tests. By this time, I have a fairly thorough understanding of the code, with all its quirks, and on the fourth pull request, I might make changes that affect the bigger picture.

If someone tried to skip straight to the fourth pull request, I would argue strongly against it, whereas I've never seen anyone argue against a pull request that just adds tests. If they won't let you make a high-risk change, make a low-risk change that moves you in the same direction.

Answer 3

It depends on your definition of "correct practice".

I'm currently working on said old spaghetti code, much of it is old enough to drink. It's a critical safety system.

Changes to this code have to be audited and signed-off by an independent third party. The owners of the company consider it to be "correct practice" for the company not to budget money on re-factoring this code because it's working the way it is and has been for years.

And yes, some of the code is pretty much garbage, but it's well-tested and trusted garbage. We don't disassemble and re-assemble rivetted bridges just because nowadays nuts and bolts are best practice.

Also, what's considered "best practice" in software engineering changes with the weather. Another 10 years and goto-spaghetti might be back in fashion ;)

Answer 4

I once had the joy of watching someone “refactoring” some legacy code that I had written, about two years earlier. My code was complicated, because it covered about two dozen corner cases that had been found through intensive testing. Each corner case handled was heavily documented.

The new code was a beauty. It was a quarter of the size, and very easy to understand. I took an old version of the code for reference and tried the first corner case. It didn’t work. I showed the new developer and he couldn’t understand why it didn’t work. I showed him exactly the code which made it work and which he had removed, ignoring the comments. Then I tried the second corner case and it didn’t work. I left him to it. I was on a different project now, and that was between him and his manager. Never checked what they did.

Now that happened with the author of the original code still there. Now imagine ten year old code, where the original author is gone, the guy working with him is gone, the one picking up after him is gone, and nobody knows what the code does and why.

I’m not saying you can’t refactor it, but you need someone who is really, really really good or you will break things in fantastic ways.

Answer 5

How do you define 'correct'?

I have seen more disasters when people tried to fix something that was not broke by fixing spaghetti code.

There are times when you will be forced to make changes.

Some folks try to totally rewrite it 'correctly'. Those attempts usually take longer and have more problems than they estimated; and sometimes they fail completely.

I have been there, done that, and I say if it ain't broke, don't fix it. Leave it for the next schmoe to worry about after you have moved on to a better job somewhere else.

Answer 6

You need a reason to be changing any code. That reason may be as simple as dead code (unreachable), making it easier to read (loops to LINQ for instance) or as complex as refactoring multiple modules for multiple interrelated reasons.

When refactoring important and large blocks of code, the first thing to determine isn’t how desirable or even necessary the change is, the first thing you want to determine is how safely the change can be made. The more important the code is to the business the higher the standards for safety. An existing bug has a known cost associated with it, and it hasn’t yet been fatal to the organization. You don’t want to replace it with a bug that drives you out of business.

A procedure that is several thousand lines, and is extremely valuable to the organization (and not just initializing a bunch of ui controls position and values), is not something that should have more than trivial changes done without first considering the risk.

That said, the first change I would consider making for such a large procedure is breaking it down into sub procedures, if it’s 3500 lines, and you can turn that into 35 sequential procedures: proc1, proc2..proc35, I would consider that a win.

In short, the decision to leave it alone or not is not directly related to the code quality, it’s a function of weighing risk vs reward. If you can eliminate the risk, then the reward is irrelevant, conversely, the risk could be so great as to again make the reward irrelevant. In most cases the risk will be moderate and the direct benefits will be minimal. Which is why the changes get nixed. If you want to make changes, work on making them risk free. Smaller procedures, unit test, adding documentation so it is better understood and less opaque.

Answer 7

In addition to the good answers I'd like to add something from my experience.

A rewrite is not always feasible for different reasons, for instance if you need to still add new features continuously. So if you need to take a step-by-step approach for refactoring you will have to accept that there will still be legacy code in place for a long time (or even forever depending on the size of the codebase and lifetime of the software).

So most of the time you have to make trade-offs where to invest your time when performing refactorings because a huge code base cannot be refactored at once.

In this case I would be pragmatic and start with refactoring in places where the highest benefits are expected. This could be, for instance, extremely buggy parts of the code or parts of the system where changes happen a lot.

If changes happen a lot refactoring of that part can provide great benefits as clean code can easier be changed than messy code.

Code that does not change often and is considered correct (i.e. does not have known bugs) should be touched at last on the refactoring journey. Or maybe it will never be touched at all...

I think most companies want to avoid unnecessary risks. As refactoring legacy code can be risky the risk should be taken for the right reasons and for code where it pays off at first.

Answer 8

From my experience, a long spaghetti code is a result of

• reinventing things that can be done easier, e.g. using built-in features. I once audited a farily big Javascript app where a lot of existing Javascript core features were implemented using custom functions and even these were used inconsistently.

• reinventing design patterns that have their idomatic implementations or should be based on well-known specs. I often see DI containers done from scratch in a way you need a while to realize what it's all about. Another example - the Observer pattern, reinvented almoast as if it's there in every Joe's subconsciousness. Another example - SSO protocols, it's been like 15 years when SAML was proposed, OAuth2 and OpenIDConnect are here also for a while but no, it's "Joe did it, he doesn't work here anymore and we are afraid to touch it"

• not following SOLID GRASP recommendations. Design Patterns? Nope, it's worse. It's like a 3000 line of code refactored to 30 methods of 100 lines named almost like Foo1 to Foo30 where each FooN calls FooN+1 in its last line

• having zero or not enough unit tests to cover basic and corner cases so that you could pretty much do whatever you want with the code and just look if tests pass. Instead, with insufficient tests, people are afraid that the code does weird things in corner cases and someone relies on these corner cases. And still somehow people are afraid to recreate these cases in unit tests so that they could refactor the code.

There's always quality vs cost/time and it's not that you should take care of everything. But any critical part could easily be isolated, tamed with unit tests and then, refactored. A good rule of refactoring is refactor until you are satisfied and then change each new bug into a new unit tests that covers this buggy case. With this in mind, I did at least couple of nasty refactorings where something completely messy turned into something that it totally under control.

Answer 9

I agree with the answer of Karl Bielefeldt, there is just a small point to add to it.

In a massive monolith of spaghetti code often a partial refactoring is impossible. Bits of code are so tightly coupled that one change in one point requires one or many changes to align the code in other points in a chain of changes so big that you easily go beyond what is reasonable in the modern agile process. So to change the code people prefer to wait for a change justified by the business in some way.