Comments are a source of contention with many developers. I don’t think that is a shocking or contrarian viewpoint. Generally people have opinions that fall into one of three camps.
- Comment everything that is not obvious
- Comment nothing. Comments are always wrong
- I don’t care, I wasn’t going to read your commentary anyways
The problem with dogmatic extremes is that they are always wrong in some way, and strictly adhering to them only guarantees that your colleagues enjoy your presence less and less, if at all.
Stale comments
One of the most common anti-comment sentiment is that as they age they become less accurate. While that is true, good comments do not state what code is doing, instead they state the reason why the code was added. Documenting the intention is always far more valuable than trying to rephrase what the code itself is already saying. Intentions are less likely to change over time. When they do it, sends a clear message that something is out of place and requires deeper inspection.
Good code documents itself
Eventually you will encounter someone spewing the platitude that good developers always write code that is clear on its own and requires no additional commentary. While writing clean and heavily tested code (you know it.. the other form of internal documentation) is all well and good, pretending that we all write magical masterpieces is straight clown shoes nonsense.
We all have rough days, long nights or other things going on in that life thing that takes place outside of the 9-5. Sometime the kindest thing to do is leaving a note for the next poor soul that has to try to make sense of why that backwards looking conditional is in fact the right thing to do after all.
The sad and lonely TODO
The intention is good. As you are working on a ticket (because you always work from a ticket) you realize there is some logic path left hanging, or some unrelated feature that could derail your progress, or just something you’d rather not do right now. That’s when you leave that special comment sprinkled in your code.
// TODO: Come back to this later and ensure the bits get stuffed
// Absolutely do this before shipping!!!And with that, you have now doomed us all.
When doing security audits, one of the very first things I do with
a code base is to grep for all instances of TODO. They are nice
little bookmarks for traps that were abandoned as quickly as they
were noticed.
Whenever doing a code review for a colleague, always do the same
search and be brutal as to why they have added a TODO in the
code. For my own code reviews, I check to see if any are present
and auto reject, as I use it to mark a blocker that needs to
be addressed before the pull request can be accepted.
TODOs with a purpose
Now, suppose there is a TODO in the code, and the reasoning
for not dealing with it right now is valid.
- The problem is orthogonal to the active ticket (one task per ticket, right?)
- The problem relates to a different bug or feature that has already been defined
- The scope of the current ticket would expand greatly based on the task
Each of these are valid reasons, keeping those provide a service to your team.
The solution here is simple, prepend every long lived TODO with a ticket identifier. Create that ticket, write about all the thoughts that you have regarding why this needs to be done some day.
// FEATURE: #4109
// TODO: Come back to this later and ensure the bits get stuffed
// Absolutely do this before shipping!!!What this has done is is give a common location where detail can
be both expanded on and cross referenced. Putting FEATURE before the
TODO also makes it cleaner to do a regular expression search for
both correct and kill on sight TODOs. Doing this shares with your
team in a way they are more likely to see that there is one more
thing that will be a nagging stone around your neck until the end
of time. Because software!