Don't Comment Your Code
OK, I used that entry title to get your attention. What it really should say is "Before you add a comment to your code, stop and ask yourself why you think you need a comment here." Clint Willard has an interesting blog entry today on code comments, and I wanted to expand a bit on what he had to say. I agree with Clint that comments are mostly unnecessary in a clearly coded application.
In the vast majority of cases, a comment indicates that something is wrong. If you have to add a comment to explain what a block of code is doing, that means that the code is too complicated or is using poor variable names or function calls.
Consider this code:
<cfoutput>
<!--- Replace the dots with slashes, then replace the root mapping name with the absolute path from the web root to the application. --->
#ReplaceNoCase(ListChangeDelims('sitemapping.components.services', '/', '.'), 'sitemapping/', '/mysite/')#/
</cfoutput>
Without the comment, someone looking at this code would probably be rather confused, or at least require them to take the time to read over it several times to dissect it and figure out what it is doing. The comment helps to explain what the code does. But this is still an undesirable outcome. In cases like this, one should consider moving the code into its own method and using good naming to convey what the code does. Something like:
<cffunction name="convertPackageToPath" access="public" returntype="string" output="false" hint="I convert a package name into an absolute path from the web root.">
<cfargument name="package" type="string" required="true" />
<cfset var path = ListChangeDelims(arguments.package, '/', '.') />
<cfset path = ReplaceNoCase(path, 'sitemapping/', '/mysite/') />
<cfreturn '#path#/' />
</cffunction>
<cfoutput>
#convertPackageToPath('sitemapping.components.services')#
</cfoutput>
Now we have much more self-documenting code. Someone coming to this line of code can understand what is going on immediately. And if we spend a little more time to make the values in the method more dynamic, such as the mapping name or the site root, we gain even more benefit. The method becomes a nice, generic option for a wide range of use cases where we want to convert package names into web-root-relative paths.
That said, of course comments have their place. They can be a good place to explain why something is happening. And they are fine for header information, documentation, or TODO entries that will be picked up by the IDE and added to your task list. But the main point is that self-documenting code should be the goal. So the next time you write or run into code that you think needs comments to explain it, consider refactoring to remove the complexity and make life a bit easier for yourself and anyone looking at the code in the future.
"I'm not a big fan of making things shorter just to save typing. That's one of my complaints about jQuery. I like the power of it - but sometimes I almost wish it had a bit more code to it to make it readable. (Blasphemy, I know.)"
Making code more readable is a great goal. Especially when you're going back to 1 and 2 year old code to make changes. When things are readable, it is all the easier to do those updates.
The problem I found (probably from habit) is that I kind of like comments to break up blocks of code. I.E in one of my gateway methods I set two composed objects into a Transfer object. setUser() and setThread(). The comment is simply ("Set the user and thread objects").. So it's pretty useless really as the code does speak for itself.. However, without the comment the code kind of looks abit bare to me.. Those statements just sitting there on their own.. lol.
I think I need to get used to not only structuring my code to make it clearer but to cut down on obvious un-needed comments.. Might take some time to break that habit I think.. :)
IMO, both comments miss the mark a bit because neither tell you *why* you're doing what your doing--which I think is most important point to convey.
Your UDF hint is much better than the inline comment, but what's most helpful is saying "why" you're doing something. That's much more helpful in the long run, as it helps you determine whether blocks of code are still necessary or even what the intent of the code is for.
I can't tell you how many times I've looked at comments from other developers that just spelled out the logic in an if statement. While that can be useful for really complex IF statements, it's much more useful to tell state *why* you're performing the conditional check in the first place.
Either way, the developer has to thoughtfully consider what they are doing. And if they're already thinking hard about what is going on, I'd say go ahead and refactor to make things more clear (and often reusable) instead of keeping track of comment updates along with every code change. Just my view though! However we try to explain things, there is always a danger than someone may misinterpret or misapply the idea. Hopefully between the blog entry and comments, we can minimize any problems. ;-)
With regard to the hint, again, I use hints primarily for API documentation generation. So it's somewhat difficult to give a "why" value to the hint since it wouldn't make much sense to someone looking at a PDF of the components and their methods and arguments. Maybe I'm missing what you're trying to say?
I also hate the comments like "loop over the array" in front of a cfloop tag. It's completely pointless. And again, for complex if statements, once I go past two or three conditions, I pull it out into a method that explains what it is actually doing (i.e. isPackageReadyToSend()), which often covers the "why" as well. If the method name is clear, there's often no need to further explain why it is being used.
Thanks.
That is also a very valid point. I think this is such a sticky topic because really the best advice is "be a thorough programmer", and that's not advice that can be followed with ease :)
Cheers.
I also really like the idea of breaking out logic-based code into its own function with a descriptive name. Even on small things, I could see that being really useful.
I'm doing a lot more 508 stuff and find as my tags become more verbose - tables with 'summary=', etc, I need to comment things less.
Obviously you can't really use a "why" with a UDF hint. However, you can still use it on the line that calls UDF:
// convert the package to directory path so we can find all the children components in that folder
sPackageFolder = convertPackageToPath('sitemapping.components.services');
While this example isn't the best, it shows off what I'm talking about.
When I'm looking at code written by another developer or even code written by myself in the past, it's more helpful to know why the code was written in the first place, not just what the particular line is doing.
I mean down the road, maybe there's no longer the need to read in all the components to find their children. When you're comments tell you *why* you're doing something, it makes it easier to update.
The why's are extremely important in some cases, such as checking for dependencies that may or may not exist. Defining how they might exist and why we need to check for them is extremely important.
targetFolder = fileUtils.convertPackageToPath(path);
targetFolderFiles = fileUtils.getFilesInFolder(targetFolder);
remoteService.sendFileCollection(targetFolderFiles);
But something like this probably would:
targetFolder = fileUtils.convertPackageToPath(path);
targetFolderFiles = fileUtils.getFilesInFolder(targetFolder);
// we have to alphabetize the files because the
// remote service requires them to be in alphabetical order
fileUtils.alphabetizeFileCollection(targetFolderFiles);
remoteService.sendFileCollection(targetFolderFiles);
So I'd add comments not only to specify why, but to denote information that is not (and really could not) be readily apparent from the code itself. But everyone has their preference of course! As long as the code is clear and easy to maintain, the comments are mainly just helpful extras. :-)
i think what it comes down to is "comment when necessary". i know brian's point goes beyond that into refactoring code into more understandable (and reusable) components, and i agree with that.
but for the most part... i think it's just apply comments when comments are justified. sean's 'coding standards' doc from a few years back addresses this as well. really no need for the following:
<!--- loop from 1 to 10 --->
<cfloop from="1" to="10" index="i">
coincidentally enough, i came across a CFC yesterday that had about 12 cffunctions in it. each function had the following over it:
<!---
*************************************************************************************************************************************
Plan Conflicts
*************************************************************************************************************************************
--->
(that one was over the 'getPlanConflicts' function).
while it's arguably pretty, i don't particularly need ascii art in my code. to be fair, a couple of them do include one liners that indicate what the function does... but yeah, i'd have put that into the "hint" attribute of the <cffunction> tag.
comes down to this for me... use consistent and descriptive naming conventions. use the "hint" attribute. when the situation dictates, use type checking (if you're on CF8, turn this off in production). all of these things should really make the code fairly self-explanatory.
when necessary, add a line or two to indicate what the code is meant to do. if you need more than 2 lines for a comment (as a general rule), it may be time to refactor.
I used to belong to the "use comments to tell the *why*" school. But lately I've realized *why* comments should go in version control commits.
a dinosaur stuck in a tarpit. Despite the ability to lift any single leg the
dinosaur is hopelessly trapped. It's not the small things that matter, its
the overall perspective.
I believe you're confusing the ability to understand code "in the small"
with understanding code "in the large". Just because I deeply understand
the driver for my NIC card, it contributes almost nothing to my ability to
understand the TCP/IP performance and functionality in Linux, even
though the NIC card code plays a vital role.
I agree that well written code blocks need little, if any, documentation.
What IS needed is pervasive documentation. That is, Knuth's idea of
literate programming. Documentation should be the point and the code
should be a smaller, integral part of the literate document. I don't care
if you've got clever, efficient code to do text substitution. I DO care
why you need to do text substitution and how that fits into the overall
understanding of the project.
So, code doesn't need to be documented.
Documentation should include code to implement the ideas.
Literate programming is needed.