Hacker News Reader

Comment by vowelless | Hacker News Reader

This should not be used. I would reject the code review if this came by me and recommend using ifdefs.

Compilers are not uniform in their interpretation of nested comments.

I'm going slightly off-topic here, but I have to rant a bit on code reviews.

For the love of god, please, they are meant for checking each others code for mistakes and not for miniscule details like this. I have seen it time and time again in various companies where code review is used as some sort of tool to dictate style and preferences to each other. Wasting valuable development time and creating a unhealthy tension between developers.

While I agree with you in general, in this particular case, we can run into problems when compiling the code on different platforms. Many teams/companies probably don't have a need for that. But I have been on cross platform projects where such errors would lead to days of wasted debugging time.

If code reviewers were just for checking mistakes, then unit tests would be enough.

Code reviews are invaluable in education other developers about your code, how to support it and how to develop on it. But information should also flow the other way and the give the reviewers the opportunity to present the accepted practices to the developer. In the long term, this saves developer time as the code base stays coherent. Code is read many times and needs to be understood by many developers even though it is written only once.

Ironically, that's the exact opposite to the way it works in my company.

We use various automated techniques to pick up mistakes and use code review to ensure style is consistent.

Everyone in the company should be able to parse any piece of code without individual styles disrupting that.

The only tension occurs around the weighty question of line length - coding standard says 76 chars max, I say that's what the IDE's for.

I think 76 characters is a good ideal to strive for because it means the code is legible everywhere, including my phone. It also reduces variables named request_response_header_validating_auditor.

However, some things don't break well, so readability ought to trump.

If you are compiling on multiple platforms, then stuff like this is a disaster.

If you are not compiling on multiple platforms, then complaining about stuff like this because of compiler issues is a nuisance. (You could still object on the basis that it is fugly.)

Spraying horrible garbage cleverness in code, and submitting commented out code, are exactly the sort of newbie mistakes that code reviews should sort out.

At best, this gimmick is for hacking out some temp code while debugging, and should not be submitted.

Of course, one should just use an IDE, or even vim, to commenting out code as needed.

If that's the only issue, you can do:

    /*/ Version one, currently commented
    /*/ Version two, currently active /**/

And a single star switches from version two to version one.

But I agree this shouldn't be left in production code; it's just a handy trick for switching between two blocks of code†. An #ifdef is also good, but that's C/C++ only.

† Whoever suggested that some editors can comment a block of code missed the point utterly - which is to switch between code blocks easily.

The trouble with this is that you now have to switch "commented" to "active" and vice versa, when I think the aim was to have a single-char switch (although the syntax is pretty horrible, to be honest).

You would just leave out the comment text.

That one also has a single character switch – just add another star on the first line. It avoids the nested comment issue of the version posted.

Every text editor can uncomment a block of code with a macro. Why on Earth would the number of chracters matter?

And every language has the IF construct.

And of you are flipping between two implementation so frequently, your actual problem is that you have no idea how your program works.

It's also unreadable unless you know the trick or see it, making future maintenance by others difficult. Clear and simple is usually better than complicated and tricky.

Reply on news.ycombinator.com