Delete your code | Hacker News Reader

Delete your code

anton-pirker.at

27 pointsantonpirker48 comments

Strongly disagree with simultaneously deleting branches and commented out code.

We can (and should) safely delete commented code because we have branches which track the evolution of features and random experiments that might someday be useful. So long as the branches exist, we can always recover that code we just deleted.

But there's not value in deleting old branches. They represent the history of a project's development and should be respected as such.

If you have a naming convention for branches, so you see what branches are expertiments or dead maybe.

But I think having a lot of dead experiment branches is like having a huge back log. It just some place where ideas go to die.

You stumple upon them once in a while and start thinking about why you never finished them and about how this was a good idea but management killed it and so on. I think it is better to delete them and set your mind free so you can focus on the tasks at hand, not the past.

Deleting branches from the version control system is heresy. It's there precisely because you know your decisions are fallible and you might want to go back in time to fix them.

If an experiment that was branched out but abandoned and forgotten, but still possibly useful, then I think it's better if it gets an entry in the documentation.

The source repository is a workplace, not a museum.

Just create a `bla-rearchitecting-experiment.md` in your `doc/` and paste the relevant code and a paragraph or two about what it does, why it could have been better and why it was abandoned. Now your history is in the place it should be, and not cluttering up the workspace of your successors.

That's very impractical for large-scale rewrites, which can easily touch dozens of files but still be abandoned (at least temporarily).

History belongs, well, in history.

In my experience lots of commented code stays around while version control is either underused or unused.

Commented code is easy and available. Trying to search through git history for a comment that maybe contains this word, or maybe that word is a lot more difficult.

There is some value to leaving in commented out code when you change code from something that is simple to understand to something written for efficiency. In one of my projects I have a few functions that account for most of the runtime. The original functions were easy to understand, but slow. I rewrote them using a whole series of complicated code that even I forget what it is supposed to do. It really helps me to look at the original functions to know what the hell I was tying to achieve - basically I am using simple code to document wtf code.

Is the efficiency gained enough to outweigh having that "stain" in the code?

Obviously, the answer to this question will depend on a whole bunch of different factors. If given the choice, I would personally choose code readability and simplicity over efficiency.

Would you also choose readability and simplicity over correctness? Sometimes efficiency is a requirement.

>Would you also choose readability and simplicity over correctness?

Depends what you mean by "correctness" and how you determine it. In a perfect world the correct way to do something is the same thing as doing it in the most readable and simple way possible.

If you are asking if I think you should give up readability and simplicity if the appropriate standard requires it, then yes. You should probably implement and follow the standard even if the standard is shit (looking at you DOM).

> Sometimes efficiency is a requirement.

Absolutely, which is why I prefaced my opinion with:

>"Obviously, the answer to this question will depend on a whole bunch of different factors."

If you are writing performance critical code, then simplicity and readability will sometimes (probably) have to take a back seat.

Well the 'stain' is pretty small in the great scheme of things - in my projects the commented out simple code is less than 1%.

It really depends on the project, but sometimes efficiency is important and when it is having a guide to what the code is trying to achieve in a clear and simple form is really helpful.

This works until you refactor the new code, and the commented out code becomes out of sync with the production code it is supposed to mirror. Commented code isn't checked by the compiler nor by the static code analysis tools, nor by the tests, so it can become out of sync fast and silently.

Yes this is always a problem. With my own code I always update (and test) the commented out code first and only change the complex code afterwards. Since the commented code is for my benefit I think the extra work doing this is worth the cost.

The way I work is I do any changes on the 'simple' code and only once I have something that works correctly do I try to update the 'efficiency' code. I have found if I try to change the 'efficiency' code directly then after a couple of small changes (or even one) I introduce some nasty bug. The basic approach I take is simple and working first and then any optimization second.

One thing I've done is use those original functions as reference implementations.

Then, in the tests, have a set of sanity check calls to both the original functions and the optimized versions.

If ever they differ, the tests fail.

What do you think?

This is basically what I do.

Nice, this also ensure that the reference code stays up to date. Seeing as comments that are meant to explain could just as well be a source of confusion because of neglect.

Some many times this. I absolutely hate seeing commented out code. There's a reason we use source control.

If a developer doesn't want to lose code they've deleted they can tag the revision in their repo so they can always get back to it quickly if needed.

> Some many times this. I absolutely hate seeing commented out code. There's a reason we use source control.

How is an unfamiliar developer supposed to find out that the deleted code exists?

Why should he/she want to find out?

"It worked different three months ago...". But I agree that this is no reason to gave to wade through commented out sections[1] but to simply use the powers of <vcs-of-choice> diff.

[1] Including if(0){} abominations

When the code was rewritten for efficiency. Whenever I read 'clever' code I wish I had an 'idiot' version that was written purely for me to understand.

So true. Whenever having to write a complicated code, I always keep a simple pseudo code in comments. That helps me both to understand later what I wanted to achieve, as well as help me focus on actual implementation details after figuring out he high level algorithm.

In my day job, I sometimes get told to make changes that I suspect will cause problems in some edge case which we might not see for months. It seems like the responsible thing to do is to leave the trustworthy version of the code there for the benefit of whoever has to fix the problem when the shit hits the fan.

I understand that reality is more complex than this, but

git bisect

That should identify the/your commit in (usually) < 10 tests. At which point the previous code is still intact.

git blame

and look at the problematic lines, see who changed it and when. Now look at the git log (filtered just for this file if you want) and identify the last changes to this part of code that way.

More complex than just

// timestamp, initials: Changed foo, commenting the following 100 lines out

maybe, but makes the day to day code so much more readable.

The better I get at programming, the more I realize that it's all about the balance between trimming messy code and avoiding premature optimization.

Your massive test suite will tell you if you broke something. You have a massive test suite, haven't you?

No!

Your test suite is meant to be a last safety net, there to catch you if you make a mistake. It's never something to rely upon. Your tests will never be comprehensive enough. Adopting a 'if the tests pass, all is OK' attitude, as implied by the article, is a terrible mistake.

Not in the Ruby community. And I think dynamic languages in general. Your test suite should be comprehensive enough to cover something as significant as a deleted function.

If you maintain the attitude that your test suite is a last safety net, then you will never be able to confidently refactor, which is what TDD in my opinion is all about.

Yes yes yes, your test suite 'should' be comprehensive enough. But is it? Are you sure?

Instead of using the tests for confirmation, you should be reasoning about the code that you are editing to be happy that your changes make sense. Having the test suites as well is just icing on the cake.

Verifying whether your test suite is comprehensive enough is not that hard in theory, if you apply mutation testing.

It's not that hard in practice either. Besides mutation testing (which becomes a rather slow affair when your codebase is of significant size) you can just do coverage checks. If you have a simple branch coverage, that alone can guarantee (in most situations) which methods are used and which aren't in your tests.

Actually coveragr only shows you that the code executes, it doesn't tell you if it's testef.

This was documented 15 years ago, see:

http://www.antipatterns.com/lavaflow.htm

Very cool link! Thank's for sharing!

Good read. But regarding deleting commented out code, I have always have a strong aversion to doing so even though I really want to, for fear that someone somewhere might want it.

Has anyone ever faced this dilemma before? If yes how did you guys dealt with it?

I always think: the code can be found in the repository somewhere. So if we need it later, we still have it. But in the production the commented out code it is just confusing.

I agree with antonpirker here. As long as you're using a version control system, the code isn't lost. Delete the commented out code, and leave a helpful commit message along with the deletion. That way the deleted code can be found, if really necessary.

Major do'h moment for me. :P Got so used to using a VCS that I took some things for granted.

Yes, I had the same issue and commented out the code instead of deleting it. Then I started using Git, started deleting the code instead and have never looked back.

Are you using a version control system? Of course you are. This means that you can (relatively) safely delete anything you want in the working directory, because you can go back and retrieve older versions from the VCS.

I keep the code if it helps me understand the code that remains. I always try to write code that is simple, but sometimes other consideration (speed) mean that I have to turn easy to understand code into something that is not. Keeping the original code can help me understand what the new code is supposed to be doing.

If I have stuff like that, I often add a date to the comment. If its not been used in 6 months and i see it again, I'll usually delete.

If you use an ide it will tell you if a function is being used in the project or not.

I find the cognitive dissonance of talking how every line of code is a burden, yet talking about you needing a "massive test suite" entertaining.

That test suite is also code. How about we go delete some stuff there?

Yeah, test suites can suffer from feature drag as well; you may be spending effort on updating or making things pass that are no longer relevant.

Your test suite consists of neatly arranged files containing fully independent functions, and it is separated from your production source tree.

The only reason it could ever be a burden is that the tests take a lot of time to run. In which case you could see if there's anything you could optimize, perhaps only run parts of your suite at any time.

And making sure that your tests are correct. I seem to spend as much time messing about until the tests work, as I do on the code it is testing.

> "Your test suite consists of neatly arranged files containing fully independent functions"

They're not independent. They depend on the code being tested.

This isn't a problem for well chosen & well made tests (i.e. testing precisely interfaces and behavior that are intended to remain the same from release to release).

But due to various religious practices, most of the tests I've seen aren't like that. Instead, they're tightly coupled to the implementation of an app, which makes refactoring harder, not easier.

Reply on news.ycombinator.com