5 ms·

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 nes

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.

qompiler 14y ago
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.
- vowelless 14y ago
  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.
- deleted 14y ago
  [deleted]
- aleyan 14y ago
  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.
- lotsofcows 14y ago
  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.
  - SoftwareMaven 14y ago
    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.
- danielweber 14y ago
  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.)
- Evbn 14y ago
  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.
Robin_Message 14y ago
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.
- aprescott 14y ago
  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).
  - Evbn 14y ago
    You would just leave out the comment text.
  - Robin_Message 14y ago
    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.
- Evbn 14y ago
  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.
cmicali 14y ago
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.

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.

qompiler 14y ago

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.

vowelless 14y ago
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.
deleted 14y ago
[deleted]
aleyan 14y ago
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.
lotsofcows 14y ago
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.
- SoftwareMaven 14y ago
  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.
danielweber 14y ago
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.)
Evbn 14y ago
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.

Robin_Message 14y ago

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.

aprescott 14y ago
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).
- Evbn 14y ago
  You would just leave out the comment text.
- Robin_Message 14y ago
  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.
Evbn 14y ago
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.

cmicali 14y ago

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.