There you are, refactoring an area of code, or just looking through trying to understand it, and you come across a piece of code that's just never used, there's no way it will ever get executed. However, it's clearly a dead 'useful' bit of code, you can see that, and it's well written, why, it's even got UnitTest
s. So you think, 'Perhaps I'll just leave it. After all, it's not doing any harm'
Wrong! (Sixteen-ton weight falls on head)
That 'useful' piece of code is
doing harm because it makes the code more complicated than it needs to be, violating the principle of TheSimplestCode
, since it does not minimize the number of methods.
Bad effects include
- You just spent half an hour unravelling this piece of code, when you find to your dismay that it just isn't used.
- You're looking for a bug, find the bit of code that 'obviously' does that bit of functionality and spend hours trying to work out why it does/doesn't work, only to discover in the end that it isn't called at all.
- You spend half a day refactoring a bit of code that ain't never executed.
- The code prevents refactors to change things that support both it and live features.
- The tests make the test suite run longer.
- One of the UnitTests for the 'useful' code fails and you waste time fixing it.
is just YouArentGonnaNeedIt
applied while you RefactorMercilessly
. However, not writing
'useful' code that isn't yet used is one thing, wantonly deleting
'useful' code takes a little more nerve.
is definitely one of my ExtremeProgrammingPractices
, but I couldn't see it explicitly mentioned on the Wiki which I thought was a strange omission. If it's already covered, please refactor. - StephenHutchinson
Perhaps RefactorMercilessly covers it. At least, when I'm refactoring, I tend to delete more code than I write.
Oh yes, it's a most satisfying feeling deleting swaths of code that have been made redundant by your cunning refactoring.
I suppose in a pure, from the start, XP environment where refactoring was always done properly you'd never get chunks of unused code. Perhaps that's why it hasn't been described? But if you're not in a pure XP environment, and you do come across such code, then YouDontNeedItAnymore
is a good XP practice. - StephenHutchinson
RefactorMercilessly, when it merges duplication, assumes both branches of the merge get called. If one of them does not, this fact now got hidden deeper, not exposed. Remove dead branches before eliminating duplication. --PhlIp
So I'm curious: What do you do with these no-longer-needed pieces of well-written code complete with unit tests? Do you simply delete them? Put them in some sort of ReuseRepository
? -- MichaelChermside
Yes, simply delete them. If you ever need them back, retrieve them from version control. Why would they go into a re
use repository when they aren't even used
in the first place?
Because if you just leave them as deleted information in the history of your version control, no one will ever know that they exist. This nifty routine that you wrote to convert dates into UTC in oracle (which we no longer need after refactoring this project) took somebody 3 days to write, debug, and test. If you just delete it, then the NEXT time that feature is needed, someone has to waste the 3 days again.
I use an 'attic' for this problem. By all means dike the code out of your current project, but keep it somewhere that you and others will know to look.
I believe that version control systems should work on a per-function level, ala Smalltalk's code database, and be integrated with a who-calls repository. Then it would become simple to make a guess that if there is any deleted code which does what you want, it would use functions A, B, and C; and query the VC system to see whether there is.
I love to delete lumps of code that become unnecessary due to refactoring. It's something that my colleagues hate to see though - if they spent the time writing it, then surely it is worth keeping... particularly the GoldPlating
that is added to every class in the name of reuse.
When that argument is made to me, I point them to the source control system, which maintains a history of all changes to the source, demonstrating that we can restore the gold plating later if we need it.
The remark about gold plating is interesting. Presumably, XP code doesn't have any such useless beauty; so, does XP code actually look different than conventional code? Can you see the XPness just by reading the source?
XpCode looks like it was written by someone who knew what he was doing and didn't waste any time getting it done. The surprising fact is that it is written by a team, the team didn't start knowing what they were doing and they probably "wasted" some time getting done.
So, does this mean that FrameworkBuilder?
s can't use XP? If I build a framework for doing a certain kind of programming, do I have to get rid of the parts of the framework I am not using at the moment? How do I handle it if I have a framework being used in ten different programs, do I make a copy of the framework for each program and then strip it down for each? How in that case do I handle the many versions of the framework? If I add something to one framework how do I make sure that it gets replicated into the other programs that use that facility and not the ones that don't?
Copying a framework into each program is a bad idea (at least at the source level; maybe it will happen as part of your build, but that should be automated); there should be one and only one canonical version of your framework. Everything in said canonical version should be used (but this doesn't mean every client of the framework has to use everything in the framework; merely that everything in the framework should have at least one client which uses it). If there is something in your framework and no clients use it, then you absolutely should remove it from the framework. This should be rare as you should be following RuleOfThree before adding new features to the framework - if something's only used by one client program, it belongs in that client program unless and until it's needed in other client programs as well.