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."
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.
When in doubt, comment out and document. I also like to keep a commented (disabled by comments) generic version of a function if I have to work a heavily optimised version. For future generations, and keep sanity when code is revisited.
Tomorrow is another day...
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.
Revision control is just one aspect. If you aren't the only person working on the code, or if there are other external dependencies such as you publish an API that others depend upon, then some form of automated testing will help ensure you haven't broken anything by deleting that code. Heck even if you are the sole developer then automated tests are still an incredibly good thingl.
If you are deleting code within a larger project then try deprecating the code first. Flag it as deprecated in the documentation (you do have documentation don't you?) and put log messages in the code that warn whenever it is called. After a while you can be confident that it is unused (or only used in obscure rarely called functions) and you can delete with confidence.
Revision control is your safety net, your last line of defence if you erroneously delete code that is in fact needed. It also allows you to always refer back to that code. Good commit messages will also help.
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.
... 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.
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.
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.
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
Unfortunately bitter experience prevents me from being too nasty on this one, because too many times I've been through the process of having good test policies in place, followed by senior management decreeing that in order to meet deadlines, testing and documentation will have to fall by the wayside 'just for a few weeks'. I know it would count as justifiable homicide, but I still can't afford the court time. ;)
++ Say to Elrond "Hello.".
Elrond says "No.". Elrond gives you some lunch.
Well, I am management. And I prefer to actually deal with stuff.
If the timeframe is borked, then I need to know so I can talk to the client about cutting features, postponing release, release in stages, etc. There is a lot you can do. But shoving out stuff that doesn't work or is unrelyable is something that will never be properly resolved.
If you desperately try to meet the release date and you actually somewhat succeed, then the books are closed on that release. But if you spend a lot of time after the release on bug fixing then eyebrows will be raised and your competence as a developer will be questioned. It's not your job to make management happy. It's management's job to make your job possible.
On unrelated news: Overtime is a clear indicator of management failure. Also massive overtime to meet deadlines never work and is never worth the effort. Been doing this for 15 years and I yet have to encounter one instance where this actually proved to do more good than harm.
20 minutes into the future
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.
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.
"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.
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.