In Brief: it Depends
In Details
Are you going to need the cleaned up, shiny stuff?
There are things to be cautious about here, and you need to identify the limit between what is real, measurable gain and what is just your personal preference and potential bad habit of touching code that shouldn't be.
More specifically, know this:
It's an anti-pattern, and it comes with issues built-in:
- it may be more extensible, but it may not be easier to extend,
- it may not be simpler to understand,
- last, but definitely not least here: you might slow down the whole code.
Some could also mention the KISS principle as a reference, but here it's counter-intuitive: is the optimized way the simple way or the cleany architectured way? The answer is not necessarily absolute, as explained in the rest below.
The YAGNI principle is not completely orthogonal with the other issue, but it helps to ask yourself the question: are you going to need it?
Does the more complex architecture really present a benefit for you, apart from giving the appearance of being more maintainable?
Write this on a big poster and hang it next to your screen or in the kitchen area at work, or in the dev meeting room. Of course there are a lot of other mantras that are worth repeating yourself, but this particular one is important whenever you try to do "maintenance work" and feel the urge to "improve" it.
It's natural for us to want to "improve" code or even just touch it, even unconsciously, as we read through it to try to understand it. It's a good thing, as it means we're opinionated and try to get a deeper understanding of the internals, but it's also bound to our skill-level, our knowledge (how do you decide what's better or not? well, see sections below...), and all the assumptions we make about what we think we know the software...:
- actually does,
- actually needs to do,
- will eventually need to do,
- and how well it does it.
Does it really need to be optimized?
All this said, why was it "optimized" in the first place? They say that premature optimization is the root of all evil, and if you see undocumented and seemingly optimized code, usually you could assume it probably didn't follow the Rules of Optimization didn't dearly need the optimization effort and that it was just the usual developer's hubris kicking in. Yet again, maybe it's just yours talking now.
If it does, within which limits does it become acceptable? If there's a need for it, this limit exists, and gives you room to improve things, or a hard-line to decide to let it go.
Also, beware of invisible characteristics. Chances are, your "extensible" version of this code will you up more memory at runtime as well, and present even a larger static memory footprint for the executable. Shiny OO features come with unintuitive costs like these, and they may matter to your program and the environment it's supposed to run on.
Measure, Measure, Measure
As the Google folks now, it's all about data! If you can back it up with data, then it's necessary.
There's this not so old tale that for every $1 spent in development it will be followed by at least $1 in testing and at least $1 in support (but really, it's a lot more).
Change impacts a lot of things:
- you might need to produce a new build;
- you should write new unit tests (definitely if there were none, and your more extensible architecture probably leaves room for more, as you have more surface for bugs);
- you should write new performance tests (to make sure this stays stable in the future, and to see where the bottlenecks are), and these are tricky to do;
- you'll need to document it (and more extensible means more room for details);
- you (or someone else) will need to extensively re-test it in QA;
- code is (almost) never bug-free, and you'll need to support it.
So it's not just hardware resources consumption (execution speed or memory footprint) that you need to measure here, it's also team resources consumption. Both need to be predicted to define a target aim, to be measured, accounted for, and adapted based on development.
And for you manager, that means fitting it into the current development plan, so do communicate about it and do not get into furious cow-boy/submarine/black-ops coding.
In General...
Yes, but...
Don't get me wrong, in general, I'd be in favor of doing why you suggest, and I often advocate it. But you need to be aware of the long-term cost.
In a perfect world, it's the right solution:
- computer hardware get better over time,
- compilers and runtime platforms get better over time,
- you get close-to-perfect, clean, maintainable and readable code.
In practice:
you may make it worse
You need more eyeballs to look at it, and the more you complexify it, the more eyeballs you need.
you can't predict the future
You can't know with absolute certainty if you'll ever need it and not even if the "extensions" you'll need would have been easier and quicker to implement in the old form, and if themselves would need to be super-optimized.
it represents, from management's perspective, a huge cost for no direct gain.
Make it Part of the Process
You mention here that it's a rather small change, and you have some specific issues in mind. I'd say it's usually OK in this case, but most of us also have personal stories of small changes, almost surgical-strike edits, which eventually turned into maintenance nightmare and nearly-missed or exploded deadlines because Joe Programmer didn't see one of the reasons behind the code and touched something that shouldn't have been.
If you have a process to handle such decisions, you take the personal edge off of them:
- If you test things correctly, you'll know quicker if things are broken,
- If you measure them, you'll know if they improved,
- If you review it, you'll know if it throws people off.
Test Coverage, Profiling and Data-Collection are Tricky
But, of course, your testing code and metrics might suffer from the same issues you're trying to avoid for your actual code: do you test the right things, and are they the right thing for the future, and do you measure the right things?
Still, in general, the more you test (until a certain limit) and measure, the more data you collect and the safer you are. Bad analogy time: think of it like driving (or life in general): you can be the best driver in the world, if the car breaks down on you or someone decided to kill themselves by driving into your car with their own today, your skills might not be enough. There are both environmental things that can hit you, and human errors also matter.
Code Reviews are the Development Team's Hallway Testing
And I think the last part is key here: do code reviews. You won't know the value of your improvements if you make them solo. Code reviews are our "hallway testing": follow Raymond's version of the Linus' Law both for detecting bugs and detecting over-engineering and other anti-patterns, and to ensure that the code is in line with your team's abilities. There's no point in having the "best" code if nobody else but you can understand and maintain it, and that goes both for cryptic optimizations and 6-layers deep architectural designs.
As closing words, remember:
Everyone knows that debugging is twice as hard as writing a program in the first place. So if you're as clever as you can be when you write it, how will you ever debug it? - Brian Kernighan