Slashdot Mirror
What Are the Unwritten Rules of Deleting Code?
Press2ToContinue writes "I came across this page that asks the question, 'what are the unwritten rules of deleting code?' It made me realize that I have seen no references to generally-accepted best-practice documents regarding code modification, deletion, or rewrites. I would imagine Slashdot's have come across them if they exist. The answers may be somewhat language-dependent, but what best practices do Slashdot's use when they modify production code?"
The more you can delete, the better.
"First they came for the slanderers and i said nothing."
Delete away! It's not like legacy versions aren't available. Most code should be deleted. Almost all production code is bad.
Help stamp out iliturcy.
what best practices do /.'s use when they modify production code?"
Should be able to easily revertable
I really like git, but the key thing is to keep revision history. Deleted code is then never "deleted" it's just no longer cluttering up the screen. Of course it does mean you need to actually learn how to use a version control system beyond blind forward checkins.
This is a boring sig
version control.
Who cares? You've got source control (SC) right? And you write unit tests right? If so then new code will pass the tests. If you write some benchmarks on performance then you'll know that too.
Build early, build often, build against test coverage and you've got Continuous Integration (CI). If you've got CI and SC then anyone can write new code and it will either pass the tests or it will break the build. If it breaks the build use SC to pull that crap out.
Done and done.
A fool throws a stone into a well and a thousand sages can not remove it.
Let Vigil delete your code for you when it's wrong and must be punished.
It's often good to just delete. Sometimes however I feel the need to put in a comment reminding the reader that the old functionality is gone. For example, it could say "no need to support device X here" or "input has already been validated". That's because it's not always evident to look in source code history to notice that there used to be something there.
Don't necessarily agree with this, but we comment out all older code when diffs aren't used. Commented code blocks with the date and editor's name works for small projects on a very tight deadline and this makes things easier if we want to rollback. Bigger projects do use diffs.
As always, this is going to be a case of "works best for my situation", rather than just "best" method to do xyz.
From the article:
Since I work on contract, I don't renew if management has a chronic unawareness problem and more often than not, the problem is people. Clueless hierarchies are a symptom of a bigger disease, the brunt of which, I'd rather not take onto my shoulders.
If computers were people, I'd be a misanthrope.
Unused code gets deleted no exceptions. If there's no client demonstrating its value, its value is zero. The reasons are obvious in systems that scale into the hundreds of thousands to millions of lines of code.The way I describe it to my teams is, Just-In-Time development rather than Just-In-Case. You can have a conceptual framework in place and fill it in on demand without committing yourself to an orthogonal matrix of features that 'might be useful'.
Ok so you are working on a team and deleting code. Here are some basic rules to follow.
1) Don't delete your boss's code. Just change all the function calls to go around it. If he asks you about it, say that someone else changed it.
2) Don't cuss out the guy who's code you are deleting until after you are done. You might have to ask him why he did certain things (unexpected library behavior etc).
3) Make sure the code you add to replace the old code is longer than what you deleted. That way, you can tell your boss that you added 'x' lines of code to the project.
4) Don't waste time unit testing your new code. Obviously if you have to replace the old code, then you are a better programmer than the last one. If the last programmer's code passed unit tests, and you are better, then obviously yours would pass unit tests too.
Any politeness the other programmer shows to you after you delete his code is not to be trusted. The code we write is pride to us, and deleting someone else's code is like seeing your coworkers girlfriend naked in the shower. Sure, it's not really your fault and you didn't really do anything bad. But expect some negative passive aggressive behavior in response.
There used to be written rules for deleting code. Then someone deleted them. And since we don't use version control, we have no way to get them back.
Please correct me if I got my facts wrong.
Either you use VCS, and you can' t ever delete code because you use it properly. Then the answer is: You can delete whatever you like in the latest revision. If you don't use VCS you have problems that exceed the scope of deleted unused code by a factor 1000, and you shouldn't be allowed access to ANY production code.
... for clarity
At a previous employer, there was a *written* rule for deleting significant blocks of code.
If a properly functioning block of code already in production was being deleted due to changes in business requirements, a comment should be inserted at or near the point of deletion, which mentions that some code was deleted and the original code can be found in version x.xx, and preferably a phrase saying what it did.
However, if the code was not yet released to the production system, or was being deleted because it's buggy, it was acceptable to simply delete it without leaving a comment (if somebody needed to research an old bug they could look in the bug tracker, which would show which version of the code last had the bug, so there was no need to mention the bug-deletion in the code).
---------
There is inferior bacteria on the interior of your posterior.
most of the time I want to delete old code when I've written something newer to replace it. I keep the old code there for easy reference, until I'm confident the new code works at least as reliably as the old one.
The more you can delete, the better.
Starting from the Murphy's law on programming: Every non trivial program has at least one bug
You can derive by rigorous analogy the Murphy's law on not-programming: Every non written code has exactly zero bugs
I have it - the holy grail, the key to bug free coding. If the deleted code has no bugs just restore the deleted lines and delete the rest. You will then have bug free programs.
It's more like breaking already tested code is wasting too much time...
Indeed, version control is the only real solution.
We use git at work, and some coworkers insist on "commenting out" code that's no longer needed. I insist that we should delete it. Should we ever need it again, we have version control; and with proper commit messages, old code is easier to find too.
1. Not always true by a long way, especially when you're coding for devices with limited space. It can also lead to unnecessary duplication, overly convoluted logic to avoid modifying existing code, unnecessary duplication, poor performance and unnecessary duplication.
2. Bug fixes are dangerous? Really? That could have saved me a lot of time at my job before last. :D
3. If you think deleting code causes information to be lost, you need to review your source code control policies. Now. Yes, it can be dangerous to remove crufty looking old code, because it frequently contains years worth of minor fixes to deal with bizarre edge cases, device oddities and the suchlike. But you know what? Those should all be well commented in the code, and covered by appropriate tests that will ensure your new version works too. If they're not, you might need to review some other policies as well.
++ Say to Elrond "Hello.".
Elrond says "No.". Elrond gives you some lunch.
No I feel you should comment it out for one version, or one iteration.
The problem with deleting code is that you lose functionality and information. Yes yes we have this ideal world where unit tests will ensure that the code only does what it is supposed to. Be that as it may, there is a reason...
Code that needs to be completely rewritten is crap code. We can argue how it came to be, but the reality is that every developer for one reason or another has created crap code. It is unavoidable. Crap code is code that does many things, but quite a bit of it incorrectly. It also is hard to get a grip on because of its complexity. For if it was easy to understand and easy to fix it would not require a rewrite, no?
Thus when you rewrite you are trying to simplify, and restructure. And because you don't have a handle of the original code you are going to introduce bugs in the new code. These bugs are new cases that you have not thought about and thus need thinking about. They are not critical bugs since the rewritten code is easy to understand and easy to fix. BUT these bugs need to be cross referenced with the original code. You need to see if these bugs are bugs, or actually the correct answer. Sometimes I was working on code and thought, "crap, the new code is right even though I thought this was a bug."
If you delete, and need to check it with version control you are adding time, effort and complexity of actions. And then instead of going back to do a cross reference you are going to introduce new bugs that can grow into weeds and cause this cycle of rewriting again.
I have found that often when I rewrite I learn new facets of my code and figure out the critical bugs. It is then I decide to stop the rewrite, and integrate the new code into the old code and fix things up. For the I see in the rewrite that the new rewritten code is going to introduce quite a bit of new code that is going to mean retesting, and assurance that things all work. And maybe this new rewritten code will not be better after all. Deleting the old code means me merging the refactored code, with the old code introducing even more complexity.
Commenting out is ugly, but it has made it easier to experimenting in a short order of time without causing version control havoc.
"You can't make a race horse of a pig"
"No," said Samuel, "but you can make very fast pig"
That's what I tell my team time and again. Do NOT simply use the IDE to comment code out. Delete it!
If the code was replaced due to a change then the old one wouldn't work anymore anyway. If the old code was too clever(mostly too kludgy and not comprehensible) then do away with it. If the replacement causes any issues then use diff to find out what was changed.
I'm quite brutal when it comes to purging stuff like that. And I have been known to be rather sarcastic when I find stuff that isn't covered via unit tests. But that is an entirely different matter.
Maintaining code is hardly rocket surgery but normal housekeeping. There comes a time when the old pizza boxes have to go. Ideally before the roaches move in.
20 minutes into the future
1. Good rebuttal. No, seriously, proof by dogmatic assertion is great.
Of course it causes all these things. If you're refusing to modify existing code, then you can't refactor it to handle extra cases, which is going to lead to unnecessary duplication (and probably hideous cut/paste code). It will expand the size of the codebase, because this is what happens when you add code, which will eventually cause problems on some platforms (trust me - I've been there).
2. Turn in your coding license. Now. Except for a very small subset of coders working on very specialised projects and using formal proofs of correctness, anyone who claims their code contains (or should contain) no bugs when first written is a fool, and a dangerous fool at that.
3. That may be your experience. Rolling back version control history to find out where defects were introduced can be very useful, and if the tools are good enough (which they are) it isn't exactly difficult either.
As for the idea that you can't track all dependencies from the deleted code to the whole system, if that's true to the extent that you claim then your design is screwed.
++ Say to Elrond "Hello.".
Elrond says "No.". Elrond gives you some lunch.
Comments.
Leave a comment saying something like: Version XX removed code to do YY because ZZ. The important things to comment are not what the code is doing (although that is useful) but the reasons for doing things and even more importantly the reasons for NOT doing things (eg: tried it but it was too slow).
Comments are an apparently obscure feature of every programming language. I say obscure as most of the code that I see uses it rarely except for a copyright block.
If rewrites are too complex you should split them up in phases. This is something few developers do, and something that can help you to test that the replacement code that you write does indeed what it's supposed to do. Between phases, testing is necessary - the more you can afford to test the new code, the less bugs you'll find later.
As a general rule, leaving commented code into commits that you make to the main repository is a bad practice. The general idea is that if you do things right, by the time you commit the code you should be pretty confident it does what it's supposed to do - and I might add that this is the main reason why I think rewrites are NOT for any developer. Attention to detail and thorough testing are a must.
When you commit commented code, you confuse other developers and add nothing of value to them. In most cases where I've seen devs do this, it's mainly because they are afraid they might need to roll back due to a lack of testing on their side.
And as a last note: version control is there to offer roll-back support, comments are not. It's about using the right tool for each job.
diegoT
If you delete, and need to check it with version control you are adding time, effort and complexity of actions. And then instead of going back to do a cross reference you are going to introduce new bugs that can grow into weeds and cause this cycle of rewriting again.
Spoken as a fool who doesn't use Git. Seriously, get a grip man. Version control works. I use commits as my "save" feature, I branch a codebase 2 or three times per day. I keep merges (mostly) sequential. Here's how I switch back and forth between two different branches right in the same source directory, so that I can test old "deleted" code vs the new re-write:
$ checkout old-version-name
Now I can make an out of source build of the old version... Then make a new branch in the same source directory.
$ checkout -b new-version-name
Now I can make an out of source bulid of the new version... make some changes, and then save them.
$ git add . && git commit -m "Frobbed foo to make bar comply with baz"
Need to make a change in the old version, or look at some code? $ checkout old-version-name
It's better than SVN or Mercurial because you don't have to manage a bunch of folders of different branches, the files change automagically when you switch between your local branches. My IDE alerts me if the file's I'm about to edit were changed out from under it and need to be reloaded, so if the files need reloading then that's what I do. I use the diff viewer if I need to reference the old code while editing the new, or make another clone of my working repository with the old branch if I'll be doing that a lot. Seriously, you need to learn about distributed version control -- I don't need to worry about not committing something that's not perfect yet for everyone to see, I just commit it locally, and rebase all the ugly changes so only pretty gets seen in public -- all these commits and branches are on my local machine, so I can actually USE the version control rather than be its slave.
Oh what's that? You use CVS or SVN and so you don't have a choice? BULLSHIT. Init a Git repo within the SVN checkout. Set git to ignore .svn folders and SVN to ignore .git. Make a git branch for "master" be the SVN branch you submit to the centralized repository, then you can branch your heart out and clone like there's no tomorrow, and merge into master, then make an SVN commit when you've got shit sorted. Accept that you've been a moron about version control all your life, and actually learn to use it. The Truth Shall Set You Free!
I need a moderation option for 'good points, but excessive dickishness' Anyways, I do pretty much exactly the same as you do; although in the firmware world.
You can switch branches in SVN just fine. It'll even merge local changes between branches for you without having to stash/pop.
If rewrites are too complex you should split them up in phases.
Yep. In the VCS this you can easily do this using branches, doing all your development and testing outside of the trunk and then merging in a single commit back into the trunk.
The general idea is that if you do things right, by the time you commit the code you should be pretty confident it does what it's supposed to do
Not (necessarily) with branches. You can commit early and often since you're not changing the trunk. That means your commit log becomes a great timeline of the changes being made, you can almost always find older revisions of code to revert to and if anything goes wrong on your development platform you have a recent backup, which is always a good thing. If a commit is faulty then you can either correct or revert, but since no-one else is using your branch yet your faulty commit doesn't matter.
And as a last note: version control is there to offer roll-back support, comments are not. It's about using the right tool for each job.
Yep. In my view VCS is to provide rollback/backup/history. Comments are to explain actions being done to make code easier to read/follow, and in complex sections of code to explain the functionality to make them more intelligible (as an example to explain an algorithm being used, perhaps along with pseudocode for more complex ones). Comments should let you completely understand a program if you only have the single checkout, but don't need to give you any history.
If you delete, and need to check it with version control you are adding time, effort and complexity of actions. And then instead of going back to do a cross reference you are going to introduce new bugs that can grow into weeds and cause this cycle of rewriting again.
Spoken as a fool who doesn't use Git. Seriously, get a grip man. Version control works. I use commits as my "save" feature, I branch a codebase 2 or three times per day. I keep merges (mostly) sequential. Here's how I switch back and forth between two different branches right in the same source directory, so that I can test old "deleted" code vs the new re-write:
$ checkout old-version-name
Now I can make an out of source build of the old version... Then make a new branch in the same source directory.
$ checkout -b new-version-name
Now I can make an out of source bulid of the new version... make some changes, and then save them.
$ git add . && git commit -m "Frobbed foo to make bar comply with baz"
Need to make a change in the old version, or look at some code?
$ checkout old-version-name
It's better than SVN or Mercurial because you don't have to manage a bunch of folders of different branches, the files change automagically when you switch between your local branches. My IDE alerts me if the file's I'm about to edit were changed out from under it and need to be reloaded, so if the files need reloading then that's what I do. I use the diff viewer if I need to reference the old code while editing the new, or make another clone of my working repository with the old branch if I'll be doing that a lot. Seriously, you need to learn about distributed version control -- I don't need to worry about not committing something that's not perfect yet for everyone to see, I just commit it locally, and rebase all the ugly changes so only pretty gets seen in public -- all these commits and branches are on my local machine, so I can actually USE the version control rather than be its slave.
Oh what's that? You use CVS or SVN and so you don't have a choice? BULLSHIT. Init a Git repo within the SVN checkout. Set git to ignore .svn folders and SVN to ignore .git. Make a git branch for "master" be the SVN branch you submit to the centralized repository, then you can branch your heart out and clone like there's no tomorrow, and merge into master, then make an SVN commit when you've got shit sorted. Accept that you've been a moron about version control all your life, and actually learn to use it. The Truth Shall Set You Free!
Spoken as a COMPLETE fool independent of version control.
In other words, an out-of-control fool.
Out of all the code you're working on, HOW WOULD YOU KNOW THERE'S DELETED INFORMATION IN A PREVIOUS VERSION OF ONE OF THE MANY FILES?
You can't know that unless there's something there to tell you. And you're NEVER going to spend much time looking into a comment like /* deleted old processing and rewrote. 2009/11/4, A. Programmer */
Yeah, you're going to dig through old versions of the source just to see what happened?
No the fuck you're not.
"Code that needs to be completely rewritten is crap code. We can argue how it came to be, but the reality is that every developer for one reason or another has created crap code. It is unavoidable."
I have found this to be the case for all code that comes out of TI for their MSP430 embedded processors. They use a convoluted system for their code that only a nutjob would love. I have thrown out their drivers for interfaces and rewrote them by hand just to avoid their crap. You DO NOT declare a function for your device driver in the .h file of another driver. WTF is wrong with their programmers?
Do not look at laser with remaining good eye.
A colleague of mine comments everything and deletes nothing despite the fact that we use version control. This leaves the actual non-commented code difficult to read and understand. Also, there have been times I may need to uncomment the code to use it again, but it takes too long to readapt it to work with the surrounding code that has evolved since the commenting, let along to understand exactly what's going on to make sure there aren't bugs. For me, I think deleting commented code should be based on the following algorithm: x = the amount of time to understand and/or adapt commented code for reintroduction y = the amount of time to rewrite the same piece of code if (x > y) { Delete the code } else { Leave it }
"Comments.
Leave a comment saying something like: Version XX removed code to do YY because ZZ."
Truly. You absolutly need to comment that... in the commit message.
Sometimes you need to leave a line like that in the code - else some programmer in 5 years time may attempt to put some more code in that does the same thing; being completely unaware of why that was taken out previously.
As a general rule, leaving commented code into commits that you make to the main repository is a bad practice. The general idea is that if you do things right, by the time you commit the code you should be pretty confident it does what it's supposed to do - and I might add that this is the main reason why I think rewrites are NOT for any developer.
This is great in theory, but often it might be someone else looking through those lines of code again, and without the direct reference to what needed to be changed before, the new developer may reinsert the bad code for one reason or another.
Another case is when a feature is put in, but then the producer or whoever changes their mind before release. It's easier for the same developer to search through all the code for snippets they remember than it is to hunt through VCS, because you can't do a "find all files" on deleted code.
There are many, many other cases for comment over immediate deletion, and I do prune my own comments once I feel confident with the change. That said, it's a bit arrogant to assume that your modifications are going to be perfect, even if you do a decent job of testing it yourself.
I guess it's worth noting that where I work, there are usually only one or two people working on code at any given time, so commits are more focused toward backup than perfection. Your environment may vary.
Charisma is the measure of someone's ability to lie with a straight face.
If you delete, and need to check it with version control you are adding time, effort and complexity of actions. And then instead of going back to do a cross reference you are going to introduce new bugs that can grow into weeds and cause this cycle of rewriting again.
The opposite of true for me. In my IDE( (Eclipse) is a single select from a drop-down menu for me to get a side-by-side comparison of new and previous, with new editable and old copyable. If I leave commented out old code, it is a scroll-up/scroll-down to compare new and old, which is much harder.
You need tools for the job. Version control is part of it, as has already been mentioned, and version control aware editors another part of it.
Consciousness is an illusion caused by an excess of self consciousness.
You need tools for the job.
Trouble is, we have too many "tools" doing the job.
I use SVN and Mercurial, but I'm much similar in my heavy dependence on version control. That's what it's for. I often have branches that live for a couple of hours just to try things out. If I ever need to come back to it, it's all in the repository -- no need for me to keep code elsewhere, possibly to be lost -- the code isn't cheap, even if it's a "throwaway" experiment. I tend to do many small commits, each of a self contained single-purpose action done to the code -- exactly as if it were a save feature. I had one 24h session where I made ~60 commits. I also use version control for electronic/industrial automation designs, and there, with small documentation revisions, I sometimes commit every 10 minutes. I can't understand why anyone would think it's too much work. It takes a couple of seconds, and documents your progress as long as you are serious about commit messages (like you well should be!).
I don't particularly mind having separate working copies for things. Yeah, the git way may be more convenient, but these days checking out a 100 megs of stuff takes a few seconds anyway. Even on a virtual machine.
A successful API design takes a mixture of software design and pedagogy.
When removing code, consider commenting why it was removed. I've seen cases where code was removed then added back in causing bugs to reappear.
Before:
Log(“Shutting down”)
CloseLog();
Reboot();
After:
Log("Shutting down”)
Reboot();
Better:
// Don’t close the log file here because some other thread might still
// write to it during shutdown/reboot, which could cause an error dialog
// that prevents shutdown. The file will be closed anyway by the OS.
Log(“Shutting down”)
Reboot();
Yes, one can and should dig through old versions of the source. Your employer paid for it, you might as well use it. Like, um, duh!
Similarly, there are those great things called GUI interfaces to version control. Heck, some even have with fancy graphs that show what files were modified and how -- including, incredibly enough, the aggregate counts of inserted, modified and removed source lines. Who'd have thought of that! Never mind the branch/merge graphs, etc.
And, of course, viewing the diffs of larger commits is just so damn hard, right?
The parent post is some bumbling idiocy, it seems.
A successful API design takes a mixture of software design and pedagogy.
But that provides -- literally -- no value.
You lose exactly the same functionality as you do when commenting it out, and you lose no information at all if you are using version control, so neither of these concerns is a valid reason to prefer commenting-out over deleting.
Unit tests aren't relevant here. The relevance is that deleting code from source files doesn't delete it from version control history, so if for some reason you need to recover the code, its still there.
Not necessarily. "There is a better way to do it" doesn't mean the code is crap code, but sometimes the better way requires a complete rewrite. And, of course, deleting code doesn't mean rewriting code. Sometimes, requirements have changed such that previously-required functionality is no longer required, and you aren't "rewriting" the code that provides that functionality, you are simply removing it.
Again, no. Even code that needs rewritten because it is of exceptionally poor quality can be of poor quality for reasons other than mixing concerns.
That may be true of some crap code, but even where the code is hard to get a grip on because of its complexity, it doesn't mean the requirements are (even if they were for the author of the crap code, which may just have been because the requirements were unclearly or incorrectly specified at the time the original code was written, and they may be clearly specified now.)
Possibly.
Having a handle on the old code isn't relevant to introducing bugs. Having a handle on the current requirements is, and old code that is "crap code" that "does quite a bit ... incorrectly" is not likely to be the best resource in understanding those requirements.
The crap code that does things wrong isn't going to tell you if things are bugs or the correct answer, the requirements will tell you that. The crap code is, at most, going to tell you if the bugs were there before (of course, you can do the same thing by attempting to reproduce the bugs on the old build, and, in fact, if the problem is, as you suggest, that the old code is hard to understand -- and even if it is not -- the best way to tell if the bug is introduced with the code change or consistent with the prior behavior is to test with the old build, since that's a direct confirmation of the behavior, rather than what it appears from the code that the behavior would be.)
You can switch branches in SVN just fine.
And as a bonus you'll have time to go get a cup of coffee!
Note to ACs: I usually delete AC replies without reading them. If you want to talk to me, log in.
So, when you realize a year and several thousand commits later that the rewrite neglected to handle a rare and complicated corner case, what's the easy way to find the previous version in the commit tree?
That's a serious question by the way. I'm still fairly new to VCSs, but the one big lack I see (and it may just be to EasyMercurial's stripped-down gentle-intro interface) is there's no simple way to say "show me the history of this specific section of code". Now sure, in an ideal world the commit comments would let you immediately recognize the relevant commit when you saw it, but thats still be a lot of manual searching to do. If I remember correctly hg does have the ability to trace the history of a particular file, but there are plenty of situations where a file may have seen lots of changes for lots of different reasons. Is there any mechanism available to quickly pick out just the changes to, say, a particular function? Or really, even just list the relevant commits.
--- Most topics have many sides worth arguing, allow me to take one opposite you.
thats what git is for. ;)
branches are cheap - use them
Merging branches, on the other hand, can be a royal pain.
Well that took a while. Seriously, aren't there coders here who can cut through bullshit? Code modification, which includes deleting swaths of useless code and putting in something simpler, is called "refactoring". And there's been a shit-ton written about it.
The refactor vs rewrite debate is likewise ancient. See: Mozilla.
These are not unwritten rules. They're just things the author is ignorant of.
Use a version control system that ties changes to a problem ticket. All versions and version comments for a paticular problem should show in the ticket. Nothing gets checked in unless tied to a approved ticket. Example systems: Clearcase with Clearquest, Subversion with TRAC.