Agile Zone is brought to you in partnership with:

I am a programmer and architect (the kind that writes code) with a focus on testing and open source; I maintain the PHPUnit_Selenium project. I believe programming is one of the hardest and most beautiful jobs in the world. Giorgio is a DZone MVB and is not an employee of DZone and has posted 637 posts at DZone. You can read more from them at their website. View Full User Profile

Meaningless docblocks considered harmful

11.25.2010
| 9323 views |
  • submit to reddit

Docblocks (in the PHP inflection), also known as Javadoc or doc comments in the Java world, are regarded as one of the few types of acceptable comments that can be inserted in code. This style - refactoring the code to show its intent instead of simply inserting comments - accepts docblocks as a necessary evil.

However, docblocks may become as evil as comments when they're not treated as first-class citizens. The examples in this article show PHP code, but the reasoning is loosely valid for any language where docblocks at the method level are written.

Too often docblocks are generated and left there without any additional information. In other cases, they are filled mechanically with information that can already be inferred. In that case, you're not helping the programmers, which will have to maintain that docblock containing information duplicated from the method signature.

It takes some seconds and some reasoning to write a good docblock, and here are some rules of thumb on how I try to write mine. The more you take the effort to write them as valuable information, the less they will be an hassle to maintenance and to colleagues reading your code.

First principle

The Prime Directive in docblocks writing is don't tell something that is already known. If you're going to only insert empty @param annotations and repeat the name of the method, you'd better off without a docblock: the parsing tool, being it Javadoc or PHPDocumentor, will be able to already infer from the method signature what you wanted to replicate.

Second principle

And the second principle is... don't tell something that is already known. Really. If meaningless docblocks consisted of code, we would have no mercy and kill them as code that is not necessary to make the tests pass (or to accomplish a task in the application, if you prefer this definition.)

On a method called makeSnafucated insert only the JavaDoc /* make snafucated */. Never define what snafucated means anywhere. Only a fool does not already know, with complete certainty, what snafucated means. For classic examples of this technique, consult the Sun AWT JavaDOC.  -- How to write unmaintainable code


Here is a meaningful docblock instead.

/**
* Extracts the square root of $value, that is, the $result such that
* $result * $result == $value.
* @param float $value must be positive
* @return float will always be positive by definition
* @throws InvalidArgumentException if $value is less than 0 or not numerical
*/
public function sqrt($value) { ... }

Let's see why a docblock like this would be helpful.

  • the method description is different from the method name; since it will be read by someone who tries to call your method (who already knows the method name), it provides information which are not . You would be surprised by how many docblocks I encounter that simply repeat the method name in natural language (usually English). This is no-brainer but the challenge is in consistently apply this principle. Entire open source projects (Doctrine 1 for example) contain generated docblocks that say nothing.
  • the description contains no implementation details however: in the case of a mathematical function, the algorithm used for the calculation is hidden (unless it is important for performance sake.)  For all we know, this method could use PHP primitive function sqrt(), or an approximation based on a truncated series.
  • parameters state their preconditions. Don't expecte to be able to call $object->sqrt(-1).
  • the result description states a post condition which will be satisfied by the method whenever it does not throw an exception. In that case, even if (-2) * (-2) is equal to 4, sqrt(4) will follow the definition and be calculated as 2.
  • occasional exceptions are cited, with their type and the reason they will be thrown.

<code>

Another practice I discovered, but quickly superseded, is to include sample calls in the docblock. Don't use the <code> tag, which gets often verbose and is out of place.

Ask yourself instead where are the unit tests for this method? That will be the right place to exercise the method itself. Tests cannot get out of sync (if you check periodically that they produce a green bar), docblock examples often can. They are only surrogates for tests.

If the Api itself is not clear, because one of the argument is a string or an array with a particular structure, consider if you're fighting with a case of Primitive Obsession. Introducing Parameter and Value Objects is often a cure better than documenting thouroughly the giant array you should build before calling the method.

@throws

I'm used to insert @throws clause for error conditions, stating explicitly what can go wrong. In PHP, all exceptions are unchecked so @throws is fundamental to express something which is not present in the method signature.

I'm not very expert in writing @throws clauses for Java methods, since there is a throws keyword in the language to define checked exceptions. If you use unchecked exceptions which are rarely caught, as they were intended to be used, it does not do any good to cite them and putting them in @throws. If you use unchecked exceptions because you want the freedom to reorganize call stacks without maintaining throws statements, then you may find useful to state some of them loudly. Unchecked exceptions are by far more manageable in long call stacks.

Have you any suggestions for better docblocks? Feel free to add your best practices in the comments.

Published at DZone with permission of Giorgio Sironi, author and DZone MVB.

(Note: Opinions expressed in this article and its replies are the opinions of their respective authors and not those of DZone, Inc.)

Comments

Sean Sandy replied on Fri, 2010/11/26 - 7:06am

I find it useful to also document the globals and static varialbes... though you shouldn't be using too many of them... it is nice to know that they are there.

  /**
* Push an image through the smush.it api
* @global string $base_url
* @staticvar SmushIt $s The smush.it object
* @staticvar bool $smush_it_testing Whether or not we are in test mode
* @staticvar string $smush_it_test_image The image that will be used in test mode
* @param file $file The file being processed
* @throws Smush_Exception
*/
static function smush_image($file_id) { ... }

Sean Sandy replied on Fri, 2010/11/26 - 7:06am

Oh no... the comment is out of date... I should fix that up

Giorgio Sironi replied on Fri, 2010/11/26 - 11:47am in response to: Sean Sandy

Maybe sarcasm mode is on, but if you're tied to legacy code at least would be an honest way to shout out external dependencies until you refactor to something which would not explode so easily.

Stephane Vaucher replied on Tue, 2010/11/30 - 1:45pm

In your "good" example, you should be straight to the point in your @return statement, e.g. returns the positive square root of argument. If a method does not have side-effects or does not change the state of an object, @return should describe the visible post-condition. It should then be used with this in mind. @param statements should also mention when a method invocation will modify the visible state of a parameter object.

Francesco Monte... replied on Mon, 2010/12/06 - 10:27am in response to: Sean Sandy

Outdated? Try phpadd!

Comment viewing options

Select your preferred way to display the comments and click "Save settings" to activate your changes.