Hacker News Reader

Comment by danieltillett | Hacker News Reader

I am guilty of commenting out code when I am working on a problem, but before committing I remove it unless it is the simple version of some function that I made way too complex in order to improve performance. I find it easier to understand later what the complex code is supposed to be doing if I have the simple implementation in the comments.

View on HN

I suggest you move the simple version into a unit test case. Making sure the simple version and the complicated one have the same results.

Plus I like to add some perf test that shows the complicated version is faster than the simple one that I can rerun after major upgrades to the lower level system e.g. when upgrading from java 1.5 to 1.6 I could remove some complicated code because the simple one was then as fast due to an improved JIT.

Yes I also think this is what I should do to, unfortunately when I first wrote the code I spend most of my time working on I didn't write unit tests at the function level (all my tests are at the module level). I have been meaning to get around correcting this, but the task is now so huge I fear it would take me more than a year to write all the unit tests :(

IMHO every automated test helps.

Don't aim for 100% test coverage aim at using unit tests to develop faster and more efficiently and the test coverage stats will come automatically.

I personally need to improve my TDD, but I am quite happy with my test driven bug fixing approach. When working on a bug I first turn it into a JUnit or selenium test, and then fix the bug and make the test pass.

I wouldn't call the simpler implementation "commented out code", as it's doing what the best comments are doing, making complex code more readable.

Having random scribbles, alt solutions and other "shitty" comments while working on a problem is definitely not a sin if you clean them up before pushing them forward.

No... you are making your code hard to understand to others by doing that. Thats if you check it in like that. Nothing wrong with leaving it there during development.

I disagree. If I see 25-40 lines of something that LOOKS like it should be simple, it's tempting to replace it with the four line version. A comment which says, "You might think this would be better as {{4 lines}}, but it is too slow... because this/that/etc" can be VERY helpful, so that one doesn't reproduce the problems (and the time fixing) that caused the initial un-simplification in the first place.

Thats totally different than just leaving it there without any context at all. Which is what i was referring to.

An actual comment is of course helpful, but when people check in code that has been commented out it usually has no explanation at all. Thats the confusion part.

I'm talking stuff like this...(and I believe so was the author)...

//int foo = 10;

int foo = 20;

if (bar)

//if (foo > 0)

{

foo++;

}

//else

//{

//foo--;

bar = false;

//}

I do explain in the comments why the simple function was removed and why the complex version is being used. The biggest problem I have found over time is divergence of the simple version from the complex version. I try and avoid this by updating and testing both versions, but whenever you have code duplication there is a risk that they will diverge.

Reply on news.ycombinator.com