Coding standards that can't agree with themselves



  • I have the unenviable task at my company of improving one of the development team's set of coding standards. They're well known for being rather slimline and, in some cases, impossible to work out what they mean. But here's my favourite...

    Section 4.1.6 states "redundant code should be deleted, not commented out."

    Section 4.1.7 states "comment out redundant code; do not remove it!"

    David.



  • The solution is clearly to wrap redundant code in a while(false) {} loop.
    That way it's not redundant code; it's unreachable code!



  • Look carefully...  Section 4.1.6 is actually commented out, but the application you're using to view it doesn't display comments in a different color.



  • @Jaime said:

    Look carefully...  Section 4.1.6 is actually commented out, but the application you're using to view it doesn't display comments in a different color.

    4.1.6. Redundant code should be deleted, not commented out. /* We need to keep our codebase clean
    4.1.7. Comment out redundant code; do not remove it!         *                         - David    */


  • @dartiss said:

    I have the unenviable task at my company of improving one of the development team's set of coding standards. They're well known for being rather slimline and, in some cases, impossible to work out what they mean. But here's my favourite... Section 4.1.6 states "redundant code should be deleted, not commented out." Section 4.1.7 states "comment out redundant code; do not remove it!" David.

    In my old company, whenever we changed code we had to keep the old version active for a few release, which would be compiled in a condition based on an environment variable known as a "backout",.

    This enabled us to rollback changes that broke behaviour at runtime without rebuild by setting an appropriate environment variable, and we could "deploy" the environment variable post-release into "powershell" script equivalents such that they would be picked up by those using the releases.

     



  • @Salamander said:

    The solution is clearly to wrap redundant code in a while(false) {} loop.
    That way it's not redundant code; it's unreachable code!

    1. Copy code
    2. Paste code under original code
    3. Delete original (4.1.7)
    4. Wrap copy in while(false) loop
    5. Comment it out

    Whether or not to comment out the while loop itself is still open to interpretation, since it was not part of the original redundant code...



  • You people aren't thinking logically enough. The two requirements create a paradox that can only be resolved if there is no redundant code.

    Brilliantly, this standard has swept their entire codebase clear of cruft by simply defining it out of existence.



  • @fowkc said:

    You people aren't thinking logically enough. The two requirements create a paradox that can only be resolved if there is no redundant code. Brilliantly, this standard has swept their entire codebase clear of cruft by simply defining it out of existence.
    What fun is there if we thought logicly.  Personally I am for you commenting out Section 4.1.7 just as pkmnfrk described.



  • @Anketam said:

    Personally I am for you commenting out Section 4.1.7 just as pkmnfrk described.
    If you comment out 4.1.7, then you've violated the remaining requirement.  4.1.6 has to be commented out.



  • @Jaime said:

    @Anketam said:
    Personally I am for you commenting out Section 4.1.7 just as pkmnfrk described.
    If you comment out 4.1.7, then you've violated the remaining requirement.  4.1.6 has to be commented out.
    Yea I got 4.1.6 and 4.1.7 backwards.  I guess it should look like:
    // 4.1.6. Redundant code should be deleted, not commented out. - Redundant, commented it out
    4.1.7. Comment out redundant code; do not remove it!         

     



  • Our code standards actually produce worse code.

    The god-awful editor we are forced to use will happily change the colour of commented code, but one of the rules is "No code shall be commented out." and another is "Changed/optimised code should remain for at least 3 revisions of that file". So we now have "#if 0" all over the place, and of course, the editor colours it as standard code, so you've no idea whether its code thats actually used in the current revision or not, where as if you comment it out, it's plainly obvious as its in a different font.

    Then you have the requirements should as "Every basic variable type such as char, int & long must be prefixed with signed or unsigned", so we have loads of compiler warnings when we call the standard libraries which expect such parameters as 'char *', as we can only pass 'unsigned char *' or 'signed char *'.

    Gahh! Why can't I get a new line on this stupid thing any more. Do I really have to put the html in myself



  • @Cbuttius said:

    @dartiss said:

    I have the unenviable task at my company of improving one of the development team's set of coding standards. They're well known for being rather slimline and, in some cases, impossible to work out what they mean. But here's my favourite... Section 4.1.6 states "redundant code should be deleted, not commented out." Section 4.1.7 states "comment out redundant code; do not remove it!" David.

    In my old company, whenever we changed code we had to keep the old version active for a few release, which would be compiled in a condition based on an environment variable known as a "backout",.

    This enabled us to rollback changes that broke behaviour at runtime without rebuild by setting an appropriate environment variable, and we could "deploy" the environment variable post-release into "powershell" script equivalents such that they would be picked up by those using the releases.

     

     

    The company I'm working for calls it a 'BRS' for "Big Red Switch" to enable/disable changed functionality on a website.

     



  • @dartiss said:

    Section 4.1.6 states "redundant code should be deleted, not commented out."

    Section 4.1.7 states "comment out redundant code; do not remove it!"

    Guys! Don't you see it?!. 4.1.6. states what should be done, but 4.1.7. tells you what to actually do. So, no contradiction there.


  • ♿ (Parody)

    @Anonymouse said:

    @dartiss said:

    Section 4.1.6 states "redundant code should be deleted, not commented out."

    Section 4.1.7 states "comment out redundant code; do not remove it!"

    Guys! Don't you see it?!. 4.1.6. states what should be done, but 4.1.7. tells you what to actually do. So, no contradiction there.

    Maybe. But even if they contradict each other, they're not redundant. Just make sure you don't copy / paste or retype a line and you should be mostly fine.



  • @Mole said:

    The god-awful editor we are forced to use will happily change the colour of commented code, but one of the rules is "No code shall be commented out." and another is "Changed/optimised code should remain for at least 3 revisions of that file". So we now have "#if 0" all over the place, and of course, the editor colours it as standard code, so you've no idea whether its code thats actually used in the current revision or not, where as if you comment it out, it's plainly obvious as its in a different font.

    There's a simple solution to that. Take advantage of your version control system (you ARE using version control, right?) and run separate tagged release branches. The really neat-o part is that as older branches come up to date, you can just merge the contents of the newer branches. (then it turns out you're using SVN, or worse, CVS, and can't really facilitate any of that).



  • @Soviut said:

    There's a simple solution to that. Take advantage of your version control system (you ARE using version control, right?)
    Sure, in-house version control, as designed by the development manager himself. To create a new version, you right-click the 'src' directory and select "Add to archive", change filename to [project name[-R[revision].rar and the job is done. If you want to compare 2 different versions, you just unpack one of the .rar files into a temporary directory and then open the file side by side and look for changes. What could be simpler. I have shown SVN to the development manager before, but he thought it was too complex and "only meant for large teams".



  • wow... just wow

    What happens when your drive fails?

    Or you start heading down a path you decide you shouldn't and need to revert?



  • At the end of each day, each work station is copied to a NAS drive (which is basically a 2TB SATA drive in an enclosure with ethernet support, hidden behind the cupboards).

    To revert is simple. Delete your 'src' directory and unpack the previous revision from the .rar :)

    We also have an excel sheet that details each revision and what was changed in that revision and why.

    So it's actually more work than something like SVN, but try telling the manager that!

    The best fun is the conflict management. When you start working on a project, you have to rename the archive you just opened with your initials, so everyone else knows not to open it!



  • @Mole said:

    At the end of each day, each work station is copied to a NAS drive (which is basically a 2TB SATA drive in an enclosure with ethernet support, hidden behind the cupboards).

    To revert is simple. Delete your 'src' directory and unpack the previous revision from the .rar :)

    We also have an excel sheet that details each revision and what was changed in that revision and why.

    So it's actually more work than something like SVN, but try telling the manager that!

    The best fun is the conflict management. When you start working on a project, you have to rename the archive you just opened with your initials, so everyone else knows not to open it!

    It's better than VSS!


Log in to reply