Discussion on:

27
Comments

Join the conversation!

Follow via:
RSS
Email Alert
You usually spend about 10% of your time writing code, and 90% reading it. It therefore makes sense to optimize it for reading. Every additional line of code means additional time and effort necessary for scrolling and finding stuff. And, time is money.

So, you shouldn't be sentimental about the code you've wrote. It's a program, not poetry. Delete it mercilessly, and let version control handle the remains. That approach is supported rather nicely by NetBeans 7.2 . It has built in local history, and a huge button on top of the text that takes you directly into the diff screen.
On the left hand side for my ide, dead code was gone as soon as I spot it, and I look for it as well.
When I have to change any of my code I keep a copy of it on file, but create the new code with only what it needs to get the job done. That's how I was first taught all those many moons ago, and it was taught that way because we did NOT have the high powered computers we have now and we HAD to keep the code to the minimum to make sure it ran faster. Part of the training was to write a small project, get the code working, then amend it by adding things, and then amend it by deleting things, and check it all still worked as intended for the final version. I'm surprised and shocked this is NOT a standard process throughout the industry now.
That's what source control is for. You can always look at past revisions.
I was taught many years ago that ideas are valuable, but the code you wrote probably stank to high heaven. So when you delete code, don't just hit the delete key. Document what was there so that if the requirement comes around again, the next guy knows where it goes. Also cross reference to the original requirements documentation - hopefully a copy will still be in the archive.
0 Votes
+ -
On rare occasions I find it useful to leave a small paragraph of code disabled. It gets preceded by a paragraph of comments explaining why it is there along with my name and the date. It is used for historical reference. This is something version control cannot help with, because the reason for the removal is not documented, and the code is hidden in the version control system without much hope of every stumbling across it again.

Let me be clear that this is not an argument against Justin's point. There are simply exceptions to his common sense advice.
1 Vote
+ -
Testing?
Organic53 25th Sep
it is called regression testing. Any developer who removes code without it being tested should be terminated on the spot.
Not used / not reachable, therefore isn't run, there can have no impact on on execution, therefore no impact on test.

It's called knowing what you are talking about...
0 Votes
+ -
Contributr
... but for things like database schemas, *it isn't that simple*. You don't just yank columns out of a DB.

J.Ja
when deleting comments. Or the source code equivalent of comments, which is Justin's point.
0 Votes
+ -
//
andrew232006 25th Sep
I always comment it out as soon I realize it isn't being used. I go back and delete the commented code later when I'm satisfied I won't be using it.
Please don't make me suffer like that.
I get code from older systems I need to completely re-write the code comments as I figure out what is happening. Comments that should have been in the whole time.

Marking things as dead code should be handled with some kind of notation so you can removed it without deleting comments when scrubbing the code automatically.
I meant I remove commented out code that no longer serves any purpose, usually while I'm reviewing and fixing up my documentation. I don't wash away all internal documentation from my code.
1 Vote
+ -
I think there are times when commented code can be useful, although with source control there is really no need unless you want to preserve an idea in the spot for later consideration.

The issue is that you need to have some good regression testing in place so that situations like the author's don't happen (yeah, right, life is perfect, right?).

That being said, removing code is ALWAYS a pain. Let's be honest, checking for references for every little piece you remove is not realistic. the compiler might catch most of the issues but the bottom line is there is always a risk anytime you introduce changes.
0 Votes
+ -
lint is for happy
Old comments are as bad as old code for hanging around; no one wants to be the guy who presses delete, just in case it turns out to be helpful 6 months from now. I work on several projects now that are jammed full of "JL 2012-02-26 Not needed anymore since ..." type comments, and they do nothing but get in the way (especially the ones referring to code that has since been removed/uncommented/etc, as the messages are then just damn confusing). These types of comments are exactly what version control exists for; why insert dated comments all over your code when version control dates the code changes for you?
0 Votes
+ -
A program that called a subroutine which did nothing except return. Perhaps in the mists of time past the sub did in fact serve a purpose but there was no code there and no comments.

Still, I am more in favor of simply rendering unused code inert via "if 0=1 then" and leave a comment regarding why it is no longer being used. I do this because I've seen too many cases of code yanked that had to then be replaced. Worse (?) is code that is commented out by commenting out each line rather that testing a "never will be true" condition.

Of course, this all is assuming disk space is not a problem and that noone will ever care you are testing a condition which will never be true.
If it is just dead code why not skip it as fast as you can.
In other cases, using compiler directives can help.
We will have global methods like screen open, screen close, data save, etc. Many are empty, or used to have something and are empty now because it was no longer needed.
An example would be our XML blob fields, originally every screen that used them had to load and save them, when this was changed, instead of editing 1000's of instances of this methods use, the method was just emptied.
A year later, a new reason to have code at that point came up, so we just reused the method, now it has a name totally unrelated to its purpose.

The people that read our code are probably going to want to kill us.
I've been known to leave code around, too. I clean it up eventually, but just often enough I'm very glad it was still sitting there. happy
when I was a GTA. Person approaches for some help saying their assignment complied fine but it isn't producing any output. I go and take a look. Lo and Behold every single line of code is in a comment block. With an incredulous tone I ask, "Why are all the lines commented out?" The reply "That's the only way I could get it to compile w/o errors."
Hello -

IMO cleaning out dead code is good as long as the "dead" version of a method is recorded in the source control database. One strategy to avoid accidental invocation of dead code is to change it's name FROM :

theMethodName()

TO :

HIDE_theMethodName()

You could pick any name :

OFF_theMethodName()
OBSOLETE_theMethodName()
DEADCODE_theMethodName()
VERSION123_theMethodName()

The idea is that you make it impossible to use the method without informing the developer that something is amiss. This is easy in Java because you only have to change the name in one place. In other languages like C++ you also have to change it in the include file where the class is declared.

Temporarily renaming methods+compiling is also a good trick to see who depends on them. This is especially true if you are compiling against binaries that are using your code, aka you can't search the library because it's not a source file.

...cfg...
An approach to removing undead code is to hive it off into a subroutine, add a comment where it used to be which points to the "subroutine", and preferably comment out the code itself.

Another is to move it to a code file which contains only undead code (a code morgue), again as a subroutine/function, and put in a comment which points to that - better if there's lots of it.

Both approaches retain the code and a pointer to it, and remove clutter from where it no longer belongs.

A final thought is that one can delete it and add a comment which indicates briefly what was deleted and in which source version it last existed, which makes it easily findable should it need resurrection.

The only approach I haven't actually used is the morgue code file.
It's a bear to to read when a full class or module is comments. In the decades I've been working, the only time I've seen code uncommented, or put back in to use successfully, is when it's created and reused in the same part of the same development cycle. Otherwise, it just takes up space.

In the event someone decides to leave code in without comments, and use some conditional to keep it invisible, it better stil have complete unit testing on it to ensure that it doesn't break while it's in retirement. Still, if its functionality becomes useful in the future, it's best to develop it from scratch to the new needs and circumstances.

If code is retired by commenting or conditionals, it's not likely to ever be useful in its current form anyway.
When I have worked on a project where there is no version control, I first try to propose and implement a project repository. Some projects have a version control process that only allows working or reviewed code. For these I go under the covers and set up a CVS or Subversion repository on my own PC. I can check in and out files at will, broken or working. I will usually put the file in before changing. If a piece of code is being reworked, I will use #if 0..#endif to get rid of obsolete code or #if 1 ???new code???#else old code #endif and test that, put it in my private repository, get rid of the conditional obsolete code, test again, and then put it in both my repository and the project repository.

A repository policy should allow checking code in intermediate states (maybe as a branch) with tagging/labeling or whatever the version control systems calls it. To capture code that is being mangled, intermediate labels of ready for review, passed review, passed unit test, integration can be helpful. If multiple releases are being developed, there should be labels for each release candidate. Of course there should be labels for anything released to quality control, manufacturing or anywhere else (apply the labels, then build).

One little trick: have a module in the build that is compiled in each build with a timestamp (e.g. C & C++ __DATE__ __TIME__) that can be accessed from the user interface, e.g. displayed on the splash screen and available from the help about screen, or, if there is no user display, easily accessed by a debugger. This can help resolve questions related to Which version is it?
0 Votes
+ -
yes!
atoms 26th Sep
That was part that a couple commenters didn't seem to read - that the projects Justin was discussing did not use version control. Which is entirely insane. The immediate solution that leapt to mind was exactly as you described: if the project does not use version control at least set up local repository for your own work.
Keyboard Shortcuts:
Prev
Next
Toggle
Join the conversation
Formatting +
BB Codes - Note: HTML is not supported in forums
  • [b] Bold [/b]
  • [i] Italic [/i]
  • [u] Underline [/u]
  • [s] Strikethrough [/s]
  • [q] "Quote" [/q]
  • [ol][*] 1. Ordered List [/ol]
  • [ul][*] · Unordered List [/ul]
  • [pre] Preformat [/pre]
  • [quote] "Blockquote" [/quote]

Join the TechRepublic Community and join the conversation! Signing-up is free and quick, Do it now, we want to hear your opinion.