C# : I hate commented out code


Even though retaining commented lines of code in your source-base is not a good thing, it’s a fact that they do exist. Whenever I see a chunk of code commented out I first make a yucky face and then the next thing that comes to my mind is why did someone do this (was he unsure??) and in case it was required, how they hell do I find out all such commented blocks of code and clean up the files.


How you comment out code in a large project heavy impacts whether all such code will be found easily and cleaned (or in some cases un-commented). There are multiple styles people use and some of them are bad practices. The so called *advanced* editor features in the IDE aggravates this to a great deal. Some of the common styles I have encountered are listed below


Multi-line comment


This was the most common way of commenting out code in the C++ era, however this is less used in C#

/*foreach (IMotionDetector detector in list)

{

Console.WriteLine(“{0} ({1})”, detector.Name, detector.Description);

}*/


If you have a coding standard in place which does not allow multi-line comments anywhere else in your code then you might live with this. However, if you have a thousand place in the source base that uses this (as in copy-right notice on top of each file and block comments on top of all public methods)  then you’ll be in deep trouble to find this piece out. The only advantage of this method is that its really simple to uncomment the code using any editor.


The single line comment


This irritates me the most

//foreach (IMotionDetector detector in list)

//{

// Console.WriteLine(“{0} ({1})”, detector.Name, detector.Description);

//}


Any source base will have thousands of // scattered every-where, so how the hell am I supposed to find code commented out in this fashion?


This is worsened by the fact that the VS IDE has the cool (???) Edit->Advanced->Comment Selection feature which actually uses this format to comment out a block of selected code. If you are using the same editor or some other editor which has the corresponding uncomment feature then you are fine. If you are unfortunate enough to like some other editor like notepad (or is being forced to use another editor due to some constrains) then happy hunting. Go to the beginning of each line and hit del twice :(


#if <token> … #endif

#if comment

foreach (IMotionDetector detector in list)

{

Console.WriteLine(“{0} ({1})”, detector.Name, detector.Description);

}

#endif


I think this is the only style that renders commented code easy to find. You can just do a search for #if comment or some other token and get to each of these. The developers using the VS IDE will be happy because VS is intelligent enough to gray out the code enclosed in #if token when token is not defined. The only issue is that if someone goes and #defines comment. So the really safe thing to do is use #if false

#if false

foreach (IMotionDetector detector in list)

{

Console.WriteLine(“{0} ({1})”, detector.Name, detector.Description);

}

#endif


Do you really need the commented piece of code


Before keeping commented-out code you really need to think twice (maybe thrice). Almost all projects use source-control so you can always get back to the code once it gets deleted. If you decide on leaving the code, add a comment on top indicating why you commented it out and why you think you’ll need it back in the future.

Comments (11)

  1. Satyadhir says:

    I think it is good to delete the code when the business logic that it is performing is no more in use. That is true, lot of commented code creates confusion. Just as a standard, having a change log at the top of the module can sort out the problems. As abhinaba said, we can always go to source control and get the prior version.

  2. tzagotta says:

    I agree with you! No source files should ever be checked into source control with commented-out code. Unfortunately, it is too easy to forget, because commenting out code during development is pretty helpful. I also like the IDE feature that comments out code.

  3. mschaef says:

    I tend to see more commented out code in projects that use source code control less effectively or not at all. In a way, it’s a sign of timidity on the part of the developer, to leave the old code around.

    "Just as a standard, having a change log at the top of the module can sort out the problems."

    Personally, I favor changelogs in CVS comments. If they’re in the source file itself, it’s 1) just something else to maintain and 2) something else to scroll past to get to your code. (I think looking at change history is less common than looking at code, and I like to optimize for the common case.)

  4. mschaef says:

    This reminds me. Conditionally compiled code has some of the same problems as commented code. I’ve found it useful to do this, where it can be used:

    #ifdef SOME_COMPILER_FLAG

    enum { SOME_COMPILER_FLAG_CODE = true };

    #else

    enum { SOME_COMPILER_FLAG_CODE = false };

    #endif

    … later, in a source file …

    if (SOME_COMPILER_FLAG_CODE)

    {

    // whatever…

    }

    This has the benefit that the possibly disabled code is always compiled, and always syntax checked. It makes it easier to avoid bit-rot, in the case you have to have conditional compilation. Also, A decent optimizing compiler will detect the constant conditional guard and eliminate the code.

  5. Will Sullivan says:

    I always surround commented-out code that I’m not willing to nuke yet in a region. Methods that are commented out go into a region at the end of a file.

    #region Depreciated

    /*

    for(this.IsCodeThat is BeingReplaced)

    {

    ButIMayNeed.It = InFuture;

    }

    */

    #endregion

  6. Marc Brooks says:

    Amen! If you are using source-control software (and you should be), then there is NEVER a reason for commented out code. If you have a feature that might come back, then drop a label on the version that had the code and add a comment in the code that specifies the label.

    If you ABSOLUTELY must comment out code, do it with an if (false) { } block around it (not an #if FALSE block) so the code is still syntax-checked with later changes, otherwise when you do restore the code it may not compile, or code it needed may have been factored away.

  7. John Davies says:

    I comment out code when my project manager insists on a change that I know is wrong. I also date it and add his name and his reasoning.

    Eventually he’ll wonder why a feature has been removed and I’ll show him the commented out code. Then I uncomment it and put it back to how I had it originally.

    If I’m commenting out code to try something, I’ll use //? or //% or anythign like that to make it easier to find and delete later.

  8. Jody Shumaker says:

    I really don’t like the use of #if false #endif to comment out a block of code.  The nice thing of using real comments is that the code will in most editors show up with the color for a comment even if you’re viewing the middle of the  comment and the start/end points are off screen.  Most editors I’ve used, VS included, do not signify that a piece of code is unreachable because it’s #if false’ed out. Makes for plenty of fun when reading the code later.  This is of course complicated by the fact preprocessor directives get automatically untabbed so they’re on the far left,  if you’re looking at code that been indented a few times you may not notice the #if false on the left, or heck be scrolled to the right enough that its not even visible.

  9. abhinaba says:

    Jody its time you upgraded your VS :)

    I’m not too sure about VS 2003, but VS 2005 does colorize #if false marked code to indicate its unreachable. The code copy pasted above is from VS 2005 and its showing grayed out code. If VIM shows it that way

  10. Andy Wyatt says:

    Should we care that much it’s not like it’s done all the time from what you’re saying and perhaps the commented code could be looked at for future referance.

    To be honist I’m supprised you spent time writing all this!

    Suppose hate is such a strong word to use and I think developers need more flexibility on things rather that being told how they should do it because of how you hate it.

    Signle line comments are cool because you can just Ctrl+K+C (Comment) and Ctrl+K+U (Uncomment) a selection of code.  If deisnged well you can run different test senarios in one class if that’s your fancy for a system you’re making.

    I’m more learned to surrounding with a region however… sometimes that can get messy to though.

  11. Dan Anos says:

    Personally, I find Commented out code generally arrives from one programmer doing a job one way, and another replacing it with a better way, but in case that something was overlooked, the old code is left in, so that the code can be reverted at a later date.

    My feeling is that since source control should do it’s job here, and a comment such as

    "// Inefficient calculation code removed 12/12/2009 view Source control for previous version"

    That means, that should it be discovered that the old code had a required side-effect, then it could be retrieved at a later date from source safe.