Why is it wrong to comment out code and then gradually remove it to keep track of what I've already done and what remains to be done?

Question

Whenever I find out that a large portion of my code needs to be changed, either because it is incorrect or because it needs to be adapted to major architectural changes necessited by other reasons, this is what I typically do:

1. I comment out all code that I suspect I might have to change. I treat the commented out code as a sort of my TODO-list.
2. I gradually review the commented out code and uncomment parts of this code, or copy-paste them elsewhere and then edit them as necessary, or rewrite parts of this code from scratch, looking at the commented out code for reference. Whenever I think I'm done with a part of the commented out code I remove it.
3. I continue this until I can see no more commented out code.

I should note that I'm largely doing this on the personal project that I'm developing alone.

However, I was told, that I should stop doing this. I was told that instead, I should start using git, referring to old commits to see old code, instead of leaving commented out code. I was told:

Commenting out code is a bad habit that should be wiped out. You lack experience so you fail to understand that. If, in a few years, you see code of another person who likes commenting out code, you will yourself start swearing at this person. Whenever I see commented out code, I remove it in its entirety, without even looking at it, because usually such code is completely worthless. You will certainly fail to see the downsides of commenting out code in small, one-person projects; but if you find a job and keep this habit there it will be a shame.

May I ask what are these downsides of what I'm doing that I'm failing to see now?

I must say I'm not really keen on only using git to see past code. As I said, I treat commenting out code as a sort of a todo-list; while git will show me how the code used to be looking, it will fail to clearly show me which parts of code are still necessary to be reviewed and which are already done. I fear I may miss some part of code and introduce bugs.

For completeness, I feel I should add that the person I'm quoting is an experienced developer and a fan of Uncle Bob's "Clean Code" - and Uncle Bob did criticize commenting out code harshly in his book.

Answer 1

If you ultimately remove all of the commented out code, I see no real issue with this. Leaving commented code in your code base is a bad practice but that is not what you are doing if you work through it all and eliminate it. I suspect the person you are talking to either doesn't understand the process you are using or is being dogmatic.

In reality, commented code is harmless. The problem is that it's messy and makes it difficult to read. There are far worse sins but it's a very simple thing to eliminate. As the person who commented out the code, you are in the best position to determine that it can be completely removed.

A lot of IDEs and code editors understand some sort of 'TODO' syntax in comments. This is an alternate way to mark what needs to be changed. You might want to consider this since it give a little more information about what you were thinking when you marked it.

At the end of the day do things the way that results in the best outcome for you. Even if this were a team project, as long as you remove all the commented code you aren't burdening anyone else.

Answer 2

May I ask what are these downsides of what I'm doing that I'm failing to see now?

Arguably, none if you work alone and don't use version control and feel it's ok to do it this way.

In fact, without version control it doesn't matter much what you do at any point in "time" as the code is always whatever state the current file is "saved" as in the operating system.

If you use version control, and you have a load of comments as your "todo list", and you fix some and remove the comment, then repeat, then repeat etc... you then have "work in progress" code and comments saved in your revision history. This won't be an issue if you never need to roll back to another commit or even "cherry pick" later (this eg is where you take specific commits and pull them into another branch to be used). But otherwise it might be a problem.

Arguably this can be compared to hard drive software "snap shots", like Windows had (the Restore thing). If it takes a snapshot with a virus in it, you kill the virus, but then later need to roll back, you can go back to a point where the virus is present again.

This approach is also likely to be a problem when you use version control and work with other devs, as then they have to see your todo list which has no use to them. In fact, it's just clutter they have to ignore and work around. In our teams we always remove all comments, such as old code or "notes". Unless they are useful - however this is very rare because we have documentation for "how it works", and software for tracking what needs to be done (aka todo).

Also, when working on a larger project you tend to collaborate, and commit and push often, so it's possible that their branch they're working on has your TODO list if they merged your branch in to theirs. Then your TODO list is everyone's business :D

In summary, if you don't work alone and especially when using version control, it clutters up the history and can be clutter to other devs.

And, this is a personal thing in some respects, but using a codebase as your "todo list" isn't ideal really. One day you might leave something in by accident, or forget to comment it or uncomment it by mistake.

---

As with a lot of approaches to architecture, coding, and how you personally or your team works, each scenario can call for something different. So consider the downsides mentioned, and the benefits of using version control, and decide whether it works for you.

Answer 3

It sounds like your reviewer is a little dogmatic. I'm not sure swearing at someone for commenting out code is PC ;-), or even helpful ;-)

But more seriously, I think your reviewer IS right, that you should seriously consider using git (or some other source control system, but git is a sensible choice).

And that doing so may alleviate some of your needs to comment out code.

But having a TODO list within the code (whether bulleted lists or old code) - is quite reasonable in my opinion. But you may want to reconsider a bit about how you do it. For one thing - I suggest thinking about someone else reading your code. That someone else could be you, a year after you drop and rejoin a project. Or it could be someone else entirely. JUST encountering commented out code is a bit confusing. Maybe something like:

/*
 * Need this sort of functionality added back before too long:
 * .... OLD CODE HERE
 */

Personally, I lean more towards something like this:

 * TODO:
 *      @todo   Possible get rid of intermediate LRUCache_ object.
 *
 *      @todo   Find some reasonable/simple way to get
 *              LRUCache<PHRShortcutSpec, PHRShortcutSpec, PHRShortcutSpecNoAuthCacheTraits_>   sRecentlyUsedCache (kMaxEltsInReceltlyUsedCache_);
 *              Working with ONE T argument
 *              Add(elt2cache).
 ...

and I feel free to throw in 'code snippets' from old code as helpful.

Using git is hard (sadly). It will take you a while to learn, and it may feel like its not part of what you are trying to accomplish. But if you are going to program usefully, you will need to learn to do so as part of a team, and communicating with a team, and git is just how that is done nowadays. And once you've gotten to using it, you will find its a VERY helpful tool/crutch, making your software development easier.

Best of luck!

Answer 4

There are many reasons to comment out code:-

• It's not right yet, and you'll un-comment it when it's ready.
• You are temporarily commenting it out to change the behaviour while debugging.
• You're not sure if the code is needed, but don't want to delete it until you've tested some more.
• The code is needed for some versions of the software, but not this one.
• The code is obsolete, but it took ages to write and you're emotionally attached to it. Besides, it might be useful one day.

The problem comes when you put the code to bed, then come back to it a couple of years later to do some maintenance. You will find the codebase littered with commented out code. It will no longer be clear why any of it is there, and it's now just clutter.

If you use any half-decent version control tool, you can boldly delete any code that you don't need any more, safe in the knowledge that the version control system still has it stored away. A file diff between versions will reveal what was deleted. Once you've implemented version control, the only need for commenting out is for temporary stuff. If you ever find commented out code in files you aren't actually working on, you can just delete it.

Answer 5

I'm not going to reiterate why you should use source control even for one-person projects, as there are plenty of other resources out there which will tell you to do this. But one major related downside of your current approach is that if you comment out code, you hide it from your IDE (if you're not using an IDE you should probably consider it).

For example, if you want to rename a method or class, or change the number of parameters a method takes, your IDE should have a refactor option to do this which will find all the appropriate references and update them accordingly - but it probably won't search in comments.

Instead of trying to guess where you need to make changes, just make them them and let your IDE tell you where your changes have caused things to break. You can then comment out the code more surgically, and hopefully for a shorter period of time before you fix whatever broke.

Answer 6

It's bad and you should stop.

The reason boils down to attempting to do a large amount of refactoring in one go.

If you comment out large sections of code, fix a little bit and check in, then you have checked in non-functional code. and a whole load of commented out stuff that others will assume is old and can be ignored

If you don't check in often, then you are accumulating merge conflicts and not recording step by step progress.

You need to change your working practice so that if you have to stop mid way everything still works.

Take baby steps and check in after each:

• extract an interface
• write a test
• refactor a function

Don't mark off large chunks of code as 'yours' by commenting them out and take them away to work on by yourself until they are complete or you fail.

If you need to keep track of what needs to be done, use a task board like scrum or trello