Should older code be updated to use newer language constructs, or should outdated constructs be stuck with?

Question

I want to make some enhancements in some still-functional code that was written a long time ago, before the programming language it is written in grew in features. In theory, the whole project uses the up-to-date version of the language; however, this particular module (and in fact, many other modules) are still written in the older dialect.

Should I:

• Not touch the parts of the code I don’t have to touch, but write my patch making use of the new language features that make it easier to write the patch but are not used in analogous situations anywhere else in the module? (This is the solution I’d intuitively choose.)
• Ignore the fact that years have passed, and reflect the style that is used in the rest of the code while writing my patch , effectively behaving as if I were doing the same task many years earlier? (This solution I’d intuitively consider silly, but given the amount of fuss everyone who speaks about “good code” makes about keeping consistency at all costs, perhaps this is what I should do.)
• Update the whole module to use the newer language constructs and conventions? (This is probably the best solution, but might require lots of time & energy that could better be spent on a different task.)

Answer 1

It is impossible to give a definitive answer to this question, because it depends too much on the particulars of the situation.

Consistency in style of the codebase is important because it helps in making the code easier to understand, which is one of the most important aspects of maintainability.
You wouldn't be the first programmer who got cursed to hell for making the code hard to understand by mixing different styles. It might even be yourself that does the cursing in a years time.

On the other hand, using new language constructs that didn't exist when the code was first written does not automatically imply that you are reducing maintainability or even that you are breaking the style of the code. That all depends on the particular language features you want to use and how familiar the team is with both the old style of the code and the new features.

As an example, it would generally not be advisable to start introducing functional programming concepts like map/reduce to a codebase that is completely in OO style, but it could work to add a lambda function here and there to a program that didn't use anything like that before.

Answer 2

To a large extent, the code and how it looks is irrelevant. What the code does is what matters.

If you can guarantee that changing/ rewriting the code will not change what the code does, then you can go ahead and refactor the code to your heart's content. That "guarantee" is an exhaustive set of unit tests that you can run before and after your changes, with no perceptible difference.

If you can't guarantee that stability (you haven't got those tests), then leave it well alone.

No-one will thank you for "breaking" a piece of business-critical software, even if you were trying to make it "better". "Working" trumps "better" every time.

Of course, there's nothing to stop you creating a set of such tests in readiness for such an exercise ...

Answer 3

Almost anything can be analyzed in terms of costs and benefits, and I think this applies here.

The obvious benefits of the first option are that it minimizes work in the short term, and minimizes chances of breaking something by rewriting working code. The obvious cost is that it introduces inconsistency into the code base. When you're doing some operation X, it's done one way in some parts of the code, and a different way in a different part of the code.

For the second approach, you've already noted the obvious benefit: consistency. The obvious cost is that you have to bend your mind to work in ways you may have otherwise abandoned years ago, and the code remains consistently unreadable.

For the third approach, the obvious cost is simply having to do a lot more work. A less obvious cost is that you may break things that were working. This is particularly likely if (as is often the case) the old code has inadequate tests to assure that it continues to work correctly. The obvious benefit is that (assuming you do it successfully) you have nice, shiny new code. Along with using new language constructs, you have a chance to refactor the code base, which will nearly always give improvements in itself, even if you still used the language exactly as it existed when it was written--add in new constructs that make the job easier, and it may well be a major win.

One other major point though: right now, it appears this module has had minimal maintenance for a long time (even though the project as a whole is being maintained). That tends to indicate that it's pretty well written and relatively bug free--otherwise, it would probably have undergone more maintenance in the interim.

That leads to another question: what's the source of the change you're making now? If you're fixing a small bug in a module that overall still meets its requirements well, that would tend to indicate that the time and effort on refactoring the whole module is likely to be largely wasted--by the time somebody needs to mess with it again, they may well be in roughly the same position you are now, maintaining code that doesn't meet "modern" expectations.

It's also possible, however, that requirements have changed, and you're working on the code to meet those new requirements. In this case, chances are good that your first attempts won't actually meet the current requirements. There's also a substantially greater chance that the requirements will undergo a few rounds of revision before they stabilize again. That means you're a lot more likely to be doing significant work in this module in the (relatively) near term, and a lot more likely to make changes throughout the rest of the module, not just in the one area you know about right now. In this case, refactoring the whole module is a lot more likely to have tangible, near-term benefits that justify the extra work.

If the requirements have changed, you may also need to look at what sort of change is involved, and what's driving that change. For example, let's assume you were modifying Git to replace its use of SHA-1 with SHA-256. This is a change in requirements, but the scope is clearly defined and quite narrow. Once you've made it store and use SHA-256 correctly, you're unlikely to encounter other changes that affect the rest of the code base.

In the other direction, even when it starts out small and discrete, a change to a user interface has a tendency to balloon, so what started as "add one new check box to this screen" ends up more like: "change this fixed UI to support user-defined templates, custom fields, customized color schemes, etc."

For the former example, it probably makes the most sense to minimize changes and err on the side of consistency. For the latter, complete refactoring is a lot more likely to pay off.

Answer 4

I would go for you first option. When I code I follow the rule to leave it in a better state then it was.

So new code follows best practices, but I won't touch code that's unrelated to the issues I'm working on. If you do this well, your new code should be more readable and maintainable then the old code. I found that the total maintainability gets better, because if you keep this up, the code you need to touch for your work is more and more often the "better" code. Which leads to faster issue resolving.

Answer 5

The answer, like so many other things, is it depends. If there are major advantages to updating the older constructs, such as vastly improved long-term maintainability (avoiding callback hell, for example) than go for it. If there's no major advantage, consistency is probably your friend

Furthermore, you'd also want to avoid embedding two styles in the same function more than you'd want to avoid it within two separate functions.

To sum up: your final decision should be based on a 'cost'/'benefit' analysis of your particular case.