There is too much old advice in PHP. A recent case comes from the PHP Advent calendar. Eli White is a strong believer in commenting code, including inline comments inside functions.
Unfortunately, he’s at least 10 years too late. This used to be good advice, but not any more.
Up to a point, he’s right. Making code as easy to understand as possible is essential and can save a huge amount of time later. And adding comments to unreadable code is better than leaving it the way it is.
There is a better way, though. Refactor your code so it’s easy to understand even without comments. I’m just reading Robert C. Martin’s recent book Clean Code. He is very clear on this point.
Clear and expressive code with few comments is far superior to cluttered and complex code with lots of comments. Rather than spend your time writing the comments that explain the mess you made, spend it cleaning that mess.
The principle is simple: if you have an inline comment in a method (or function), take the chunk of code that the comment refers to and extract it into a separate method. Give the method an intention-revealing name. Typically, you won’t need the comment afterwards.
I’m sure we need an example. Here’s a small excerpt from a method taken from Zend Framework.
protected function _doUpdate()
{
...
/**
* Execute cascading updates against dependent tables.
* Do this only if primary key value(s) were changed.
*/
if (count($pkDiffData) > 0) {
$depTables = $this->_getTable()->getDependentTables();
if (!empty($depTables)) {
$db = $this->_getTable()->getAdapter();
$pkNew = $this->_getPrimaryKey(true);
$pkOld = $this->_getPrimaryKey(false);
foreach ($depTables as $tableClass) {
try {
@Zend_Loader::loadClass($tableClass);
} catch (Zend_Exception $e) {
require_once 'Zend/Db/Table/Row/Exception.php';
throw new Zend_Db_Table_Row_Exception($e->getMessage());
}
$t = new $tableClass(array('db' => $db));
$t->_cascadeUpdate($this->getTableClass(), $pkOld, $pkNew);
}
}
}
By changing some temporary varibles into instance variables and extracting a couple of methods, we can get code like this:
if ($this->primaryKeyValuesHaveChanged() {
$this->executeCascadingUpdatesOnDependentTables();
}
Or even, by extracting a couple of classes, making objects out of two of the concepts involved, we might end up with something like this.
if ($primaryKeys->valuesHaveChanged() {
$dependentTables->executeCascadingUpdates();
}
This uses most of the words from the original comment, but it’s superior for several reasons. First and foremost, the section of code as a whole is clearer. Second, comments have a tendency to get out of sync with the code. By expressing the message in the code itself instead, we can avoid that. Third, the extraction itself has the positive side-effect of making it possible to test the extracted method in isolation.
Improving the code itself is harder than writing comments, but it’s worth it. .
![Reblog this post [with Zemanta]](http://img.zemanta.com/reblog_a.png?x-id=61cc5cf5-e2d2-47e1-aca9-1e47fc0ad1bc)
I agree. If you make the function names and variables human readable then it goes a long way to self commented code. I only comment using doc commands so that docs can be created from the code with phpdoc. Everything else you should be able to figure out by reading the code.
$this->owner->firstChild->position->setAndRepaint($something)
should be
$this->setSizeOfOwnersFirstChildAndRepaintThatGuy($something)
sure makes more sense to me!
So… were does the harmful part come in? I agree that clean code may not require comments, but what about clean code AND good comments to it?
How are comments harmful in any way?
@nico – if you need comments to explain something, and it’s not
1) Obscure maths
2) Highly optimized code
3) A warning about broken-expectations… (don’t touch foo, even though it looks tempting!)
… then your code smells bad.
If you need comments everywhere because the code is poor, then it is much harder to maintain, test, explain and understand.
The more difficult all of those things are, the worse communication gets, and poor communication leads to failure, bugs, anger, and frustration.
What kind of code is clean enough? Most developers think their code is great and easy to understand that it does not need to be commented. Then they read your article and find that they are correct.
Please ,your coworker will not read your code first, they read your comments, especially DocBlock. Because your comments give them the idea on how to interpret the subsequent code in a correct way.
My recommendation: Improve your code first, improve your comments later but always think of code commenting. Commenting is a great way that helps your and your coworker improve the code later.
pcdinh AT phpvietnam
Daniel, I perfectly understand your code, but you’re assuming that who reads the code is a skilled programmer…
I’m a scientist and often I write code that my collegues will use and possibly modify. If the person who modifies the program is not a very good programmer it may be better to have good comments than have them coming bugging me because “my code doesn’t work anymore”…
I’ll give you an example.
This is a dumb comment
// This calculates the 3rd power of n
$res = pow($n, 3)
But if I write
// We use the 3rd power of n as suggested by Smith et. al. Journal of Something 1992
$res = pow($n, 3)
This is a very useful comment
Anyway you haven’t answered to my question:
When do comments become HARMFUL?
They don’t become harfmful. They never did and never will.
The code described here may be bad, but it’s not the comments’ fault.
What a misleading title for a blog entry. I guess it attracted a lot of people to it !
The answers to the question “when do they become harmful?” is (most importantly) when they are used as a surrogate for improving the code and (secondly) when they’re useless because they don’t make the code clearer. In this second case, they are likely to be ignored and therefore to get out of sync with the code and become actively misleading.
There are cases when comments are useful. The book I mentioned has a long list of these cases. For the case of inline comments, which was my particular target, in my experience a vast majority of them are not. You might object that that does not justify the title “Comments considered harmful”. Perhaps, but my take on the word “considered” is that I’m just considering the idea that comments may be harmful. According to the dictionary, that means I’m thinking about it carefully.
I disagree. I don’t comment much within functions but there are lots of times when a one-liner actually does help. I often comment regexes (humans should not have to read these), and sometimes comment program flow too, e.g.
// first handle the case where no records were found
(if $records == 0) {
return false;
}
The reader could of course go back and see what was expected in $records and clearly see why the block is there, but adding the comments lets them keep the thread of what they are reading in my opinion. Certainly it helps me HUGELY when I re-read my own code a few months later to understand in big-picture terms what I was doing.
To some extent its a question of personal style (and of course of coding standards), but I don’t think “harmful” is an appropriate way to describe comments.
@LornaJane: That doesn’t look like a good example of a necessary comment. If it’s hard to understand that $records == 0 means that no records were found, the easiest change is to rename the variable to something like $numberOfRecordsFound or $numRecordsFound. But typically, it’s possible and preferable to avoid this kind of check altogether.
On the other hand, I’m more sympathetic to comments for regular expressions. There’s no obvious alternative way to make them readable. I’ve discussed regular expressions earlier (see http://tinyurl.com/9wzt62).
Jo suspect only pros work on you code. Joe programmer might need one o another hint.
So as far as i am concerned i do not listen to fowler here: Self explaining names: yes. Leaving comments out: NO.
Comments should explain the “why” and not the “what”.
I don’t like using such long identifiers as a crutch. In any major project youWillGetIdentifiersThatAreLongerThanEightyCharactersAtSomePointAndThisMakesItHarderToRead. The author is trading comments for unwieldy identifiers.
A person must have basic understanding of the clases/methods/functions of code they are reading. Neither comments or expressive code will let them get around it.
@nico: comments are only signs that your code are breaking OO principles (Single Responsability, etc.) and that your code is not clear enough to be understood.
For the people who want to improve their knowledge and who do not understand why commenting is a sign of weakness, I can recommand the following articles:
- http://www.objectmentor.com/resources/publishedArticles.html
(especially under “design principles”, “Object Oriented Design”, “Refactoring”, etc.)
- Misko Hevery’s blog (A google evangelist) http://misko.hevery.com/
Comment is usefull when:
- it is used by programm to autogenerate documentation
- it is used by your IDE
- You write ugly code for external constraint (performance reason, etc.)
- …?
I use comments primarily above functions and method declarations for generating documentation. This is VERY important to me and often times is enough information for me to understand what a method/function is meant to do.
I think I’m going to start using variables that are more readable like you suggest. Often times I name my variables using the context of where they are. So an array of news posts would be $posts instead of $news_posts. Going over 15 characters is overkill though and I’ll aim to not do that.
For those disagreeing with the concept outlined by the author, try to see it for what it is and apply as much as you can. Be open to new ideas!
In my case, I would never completely remove comments from my code but I will focus on making the code readable enough so that comments aren’t always necessary. Even if it makes 10% of my code more readable since I’m thinking about the concept while writing the code — awesome!
If ever there was a posting to which the “Considered Harmful” Essays Considered Harmful posting – http://meyerweb.com/eric/comment/chech.html – applied, it’s this one.
I have read that before. I fully understand the argument and agree with it in general terms. In particular, I agree with the idea that ‘the writing of a “considered harmful” essay often serves to inflame whatever debate is in progress..’
In this case though, there seems to be no debate in progress in the first place. I am trying to make my point strongly enough that people will at least notice it.
If the rest of the blog post had been over the top, then I would probably have been shooting myself in the foot.
I don’t think it is, though.
On the other hand, the amount of interest that’s being focused on the title rather than the actual content of the blog post could indicate that it wasn’t a wise choice.
I think I’ve seen this presentation on JavaZone. The comments become harmful when you change things further down in your method-call-stack without changing comments higher up in the stack.
The presentation had some believable examples describing apparently innocent changes way down in the code witch in turn made comments higher up very wrong.
The only thing worse than (bad?) code without comments is code with comments that’s wrong!
> comments are only signs that your code are breaking OO principles (Single Responsability, etc.) and that your code is not clear enough to be understood.
No, comments are there to comment. Period.
That has nothing to do with how good is your code.
You can write perfectcly clean code and add good comments to it. Nothing wrong with that.
You can’t generalize on these things.
In certain cases a rough quick routine that maybe doesn’t follow the golden sacred rules of good programming can be faster and more efficient than one that is maybe easier to read and perfect by any standard of programming.
Sometimes you need efficiency over pretty code. And good commented efficient ugly code is good in that case.
Sometimes you have a gigantic project and you need clean code if you want to maintain it. Still, also in that case good comments are good to have.
Hello Dag,
what did you exactly do in your example? If I understand correctly, you have only created two new methods and moved “uncleaned” code into these methods.
It reminds me of “fantastic” Wordpress’ index.php:
define(‘WP_USE_THEMES’, true);
require(‘./wp-blog-header.php’);
Nice!! All garbage is gone …
What I’ve done is I’ve made the main logic easier to read (except I haven’t actually done the refactoring, just indicated how it might look). The difference between this and the Wordpress example is that it has no meaningful program logic at all.
That’s just starting from the top. Then there are opportunities for refactoring the code that, as you point out, I’ve moved into the new methods. The first thing I would do is to reverse the sense of the if statement, changing
if (!empty($depTables)) {…
into
return if(empty($depTables));
That eliminates a level of nesting.
This blog seems to attract a fair bit of critique – though a few people may have missed the authors point a somewhat. I think the issue here was with the ‘comments’ PHPadvent post (great calendar by the way..) i.e. writing expressive code should be encouraged instead of zealous commenting.
Anyway I found this post (and discussion) insightful, the key points I take away are:
- Expressive semantic code is preferable complex code with descriptive comments
- Not all comments are evil! (RegEX/DocBlock/Modified-by-someone-on-date)
- Comments could be used as ‘refactoring indicators’..
E.g. you could have a plug-in for your editor/IDE that scans your code and highlights all non-regex/Docblock comments and flags them as ‘refactoring candidates’. It might generate a ToDo list of line-numbers which you could then review and refactor as necessary.
Maybe that would be useful? I don’t know.. interesting post though, keep it up.
[...] Comments considered harmful [...]