Checking in of “commented out” code [closed]

Question

Ok, here is something that has caused some friction at my current job and I really didn't expect it to. Organized in house software development is a new concept here and I have drawn up a first draft of some coding guidelines.

I have proposed that "commented out" code should never be checked into the repository. The reason I have stated this is that the repository maintains a full history of the files. If you are removing the functional code then remove it altogether. The repository keeps your changes so it is easy to see what was changed.

This has caused some friction in that another developer believes that taking this route is too restrictive. This developer would like to be able to comment out some code that he is working on but is incomplete. This code then would never have been checked in before and then not saved anywhere. We are going to be using TFS so I suggested that shelving the changes would be the most correct solution. It was not accepted however because he would like to be able to checkin partial changes that may or may not be deployed.

We want to eventually get to a point where we are taking full advantage of Continuous Integration and automatically deploying to a development web server. Currently there is no development version of web servers or database servers but that will all be changed soon.

Anyway, what are your thoughts? Do you believe that "commented out" code is useful to have in the repository?

I'm very interested to hear from others on this topic.

Edit: For clarity sake, we don't use private branches. If we did then I'd say do what you want with your private branch but don't ever merge commented out code with the trunk or any shared branches.

Edit: There is no valid reason we don't use private or per user branches. It's not a concept I disagree with. We just haven't set it up that way yet. Perhaps that is the eventual middle ground. For now we use TFS shelving.

Answer 1

In my experience, developer switches are commented out code.

Sometimes, new back-ends are built in parallel, with the activating switchs commented out in source control.

Some bizarre feature we need once on a blue moon but no customer will ever need is often implemented that way. These things usually carry a high risk of security or data integrity bypass so we don't want them active outside of development. Requiring a developer who would use it to uncomment the code first seems to be the easiest way of getting it.

Answer 2

Another reason for checked in commented out code:

You're modifying existing code, and found a subtle bug, one that is easy to overlook, and perhaps might even look correct at first glance. Comment it out, put the fix in its place, and add comments for what is going on, and why it was modified. Check that in, so that your comments on the fix are in the repository.

Answer 3

I would prefer to see possibly broken, accessible code that isn't being used yet checked in over the same code being completely unavailable. Since all version control software allows some sort of 'working copy' separate from the trunk, it's really a much better idea to use those features instead.

New, non-functional code is fine in the trunk, because it is new. It probably doesn't break anything that already works. If it does break working code, then it should just go in a branch, so that other developers can (if they need to) check that branch out, and see what's broken.

Answer 4

I absolutely agree that commented out code shouldn't be checked into the repository, that is what source code control is for.

In my experience when a programmer checks in commented out code, it is because he/she is not sure what the right solution is and is happier leaving the alternate solution in the source in the hope that someone else will make that decision.

I find it complicates the code and makes it difficult to read.

I have no problem with checking in half finished code (so you get the benefit of source control) that isn't called by the live system. My problem is with finding sections of commented code with no explanation the dilemma was that resulted in the code being excluded.

Answer 5

I think checking-in commented code in a source code control system should be done with extreme caution, especially if the language tags used to comment the code are written by blocks, i.e.:

/* My commented code start here
My commented code line 1
My commented code line 2
*/

Rather than on an individual line basis, like:

// My commented code start here
// My commented code line 1
// My commented code line 2

(you get the idea)

The reason I would use extreme caution is that depending of the technology, you should be very careful about the diff/merge tool you are using. With certain source code control system and certain language, the diff/merge tool can be easily confused. The standard diff/merge of ClearCase for example is notoriously bad for merging .xml files.

If it happens that the commenting blocks lines are not merge properly, presto your code will become active in the system when it shouldn't be. If the code is incomplete and break the build, that is probably the least evil, as you will spot it immediately.

But if the code passes the build, it may become active when it shouldn't be there, and from a CM perspective, that could be a nightmare scenario. QA generally test what should be there, they don't test for code that is not supposed to be there, so your code may end up in production before you know it, and by the time it would be realized the code is there when it shouldn't, the cost in maintenance have multiplied many folds (as the "bug" will be discovered in production or by the customer, worst place or time ever).

Answer 6

If the developer has commented out some code because it is not yet complete, then the correct "source control" way to deal with this would be for that developer to keep it in a separate branch of his own, until that code is ready to check in.

With a DVCS (like git, bazaar, or mercurial) this is dead easy as it requires no changes in the central repository. Otherwise, perhaps you could talk about giving developers their own branches on the server if they are working on specific features that will take them a long enough time (ie, days).

There is nothing wrong with checking in commented out code in some situations, it's just that this is one situation where there may be a better way to do it, so the developer can track changes to his source even though it isn't ready to be checked in to the main repository.