Commented Code Responsibility/Etiquettes
I had a conversation with a colleague of mine that would like to use the commented code I used for trace/debug I did some time ago. They contacted me if that code will just work after uncommenting it. I said it should be okay, but please check if it works as intended; I would not like to make a statement that it will definitely work since commented code will regress over time. They wanted a binary answer of will it work or not, that makes me hard to answer their question. Does the responsibility lies upon me, who should maintain commented code, or the person that wants to use it?
If the person that wants to use it changes the code (e.g. by uncommenting something) then that's their responsibility.
On the other hand, if they are a "colleague" then, as Christmas parties have been de facto banned (at least in England, and especially if they are in Downing Street) then your contribution to workplace harmony in the festive season might be to test out what they are planning to do with the code.
On the other hand, if they are a "colleague" then, as Christmas parties have been de facto banned (at least in England, and especially if they are in Downing Street) then your contribution to workplace harmony in the festive season might be to test out what they are planning to do with the code.
IMO never, ever, ever make a categorial statement that some piece of code 'will work'!
You could make some statement such as 'As provided, the code is not known to have any adverse against its intended purpose. The user is ultimately responsible for the effects of this code and the integrity of the machines on which it is used'.
You could make some statement such as 'As provided, the code is not known to have any adverse against its intended purpose. The user is ultimately responsible for the effects of this code and the integrity of the machines on which it is used'.
no one should maintain it.
Its right there, and whoever activates it needs to be the on to validate that it still works and update it as needed. Its way, way, out of scope to crawl through every file looking for blocked off code and checking it. If the code is important, rather than block it off, you should #define block it instead, so you can activate it and test/debug it with a flag to keep it up to date.
It would be a 'friendly gesture' to observe it when updating a code block and update it / verify it as you go rather that just ignore it. That would be a policy or not at your place of work, though.
Its right there, and whoever activates it needs to be the on to validate that it still works and update it as needed. Its way, way, out of scope to crawl through every file looking for blocked off code and checking it. If the code is important, rather than block it off, you should #define block it instead, so you can activate it and test/debug it with a flag to keep it up to date.
It would be a 'friendly gesture' to observe it when updating a code block and update it / verify it as you go rather that just ignore it. That would be a policy or not at your place of work, though.
I agree with both statements, I just needed an unbiased view on this. Thank you :)
Ideally, your software project should have comprehensive unit tests.
This way code changes are way less "scary", because after the change you can simply test whether all your test cases still run through properly. If all tests do succeed, even after the code change, then you can be pretty confident (though not 100% certain, of course) to not have broken anything this time :-)
I'd say, whoever makes a code change is responsible for ensuring that none of the existing tests fail! In case that any tests fail, the code change shall not be committed, until either the new code has been fixed to make all tests succeed again, or (if necessary) the test case itself has been updated/fixed !!!
I'm not a big fan of having lots of "commented out" code in the repository. In fact, I hate that! For things like debug code better use macros, so that the "debug" functionality can easily be enabled or disabled at any time. This allows the project to be tested, in an automated way, with debug code enabled or disabled. It makes sure that the debug code doesn't break unnoticed, even though it is not usually enabled in "release" builds.
Anyway, whoever re-enables some code that had been "commented out" earlier (probably for a reason!) would still be responsible to make sure that no exiting functionality breaks. Even more important, whoever adds new code to the project, should be responsible to also add the appropriate test cases to your test suite.
I'd even vote for forbidding to leave "commented out" code in the repository. Either make that code conditional, e.g. with the help of macros, or just throw it out completely! You still have it in the history of your VCS.
This way code changes are way less "scary", because after the change you can simply test whether all your test cases still run through properly. If all tests do succeed, even after the code change, then you can be pretty confident (though not 100% certain, of course) to not have broken anything this time :-)
I'd say, whoever makes a code change is responsible for ensuring that none of the existing tests fail! In case that any tests fail, the code change shall not be committed, until either the new code has been fixed to make all tests succeed again, or (if necessary) the test case itself has been updated/fixed !!!
I'm not a big fan of having lots of "commented out" code in the repository. In fact, I hate that! For things like debug code better use macros, so that the "debug" functionality can easily be enabled or disabled at any time. This allows the project to be tested, in an automated way, with debug code enabled or disabled. It makes sure that the debug code doesn't break unnoticed, even though it is not usually enabled in "release" builds.
Anyway, whoever re-enables some code that had been "commented out" earlier (probably for a reason!) would still be responsible to make sure that no exiting functionality breaks. Even more important, whoever adds new code to the project, should be responsible to also add the appropriate test cases to your test suite.
I'd even vote for forbidding to leave "commented out" code in the repository. Either make that code conditional, e.g. with the help of macros, or just throw it out completely! You still have it in the history of your VCS.
Last edited on
Agree with kigar. Commenting out code for any long-term duration is a nasty habit and should be avoided. Old code should be kept track of through version control. Every line of code (or comment) that doesn't run is a line of code that probably has a bug in it. That being said, the person that wants to change the code by uncommenting it should be the one that makes sure that code works.
PS:
"commented code" to me means code that is alive, but has comments explaining things about it.
"commented-out code" is the less ambiguous terminology here, in my opinion.
PS:
"commented code" to me means code that is alive, but has comments explaining things about it.
"commented-out code" is the less ambiguous terminology here, in my opinion.
Last edited on
meh, all version control does is make it a hassle to recover the deleted lines --- which are STILL going to be out of date if the rest of the function was modified since that time. It saves you nothing for this type of effort, IMHO. Well, it cleans up the source a little, which has merits, but in terms of getting the old part working again, no help there.
that said, yes, macros !
that said, yes, macros !
Last edited on
| that said, yes, macros ! |
Yes. But macros only help, if the code in question is still actually enabled, for some relevant use cases.
Sadly, I have seen things like this:
|
|
If used in that way, macros (pre-processor directives) really are not better than "commented out" code.
| meh, all version control does is make it a hassle to recover the deleted lines |
Probably more a question of which diff-viewer/merge-tool you are using.
Last edited on
Topic archived. No new replies allowed.