What is the proper way to document files, classes and constructors?

Question

This is highly depending on your context and the assumed skill level of the people you work with or the people that come after you.

If you publish a framework, a library or something of the sort you usually assume that your user are of all skill levels so you might want to document as much trivial crap as possible to reduce the load of questions your community has to handle.

Let me start of with an example where I think documentation can be a major pain.

What do you absolutely need

If you want to use PHPDoc you need a file doc block and another doc block after that (usually the class doc block).

Those **need* to have a @package Tag. Everything else is optional.

I'd go so far and say that even the @package Tag is optional since you might be able to automatically generate it for project. And if i remember correctly PHPDoc lets you even set a default package for everything that doesn't have a tag.

For documentation in general let me start of with an example (a longer example is at the end):

How many times can you explain what "uri" means:

Note that for getUri it is explained what URI stands for (just to have something to talk about in the comment I'd assume) while it isn't in isAbsUri because there you can at least say "abs means absolute" twice.

---

If you are not an open source project (or need to ship COMPLETE!!!11eleven api documentation):

I'd strongly suggest to use proper, long and descriptive class, variable and method names instead of documentation.

There is no gain in writing something again in the doc blocks and since it is 2011 and we have 120 char wide terminals and autocompletion there is just no need anymore to abbreviate everything for the sake of saving some chars.

I'd even argue that documenting trivial things hurts you and your team by forcing you to waste time on things nobody gets value from you get into a habit of always writing trivial docs and not reading them anymore.

A good comment should explain WHY something was done while the code its self should explain HOW without needing further comments.

My favorite example for redundant docs is this one:

class myClass {
/**
 * Constructor
 */
public function __construct() {
}

Some guidelines say you HAVE to document EVERYTHING and you end up with people stating the obvious again and again.

This adds no value but wastes time when reading the code.

An example for proper naming:

class Person { 
/** 
 * Set a persons weight in Kilogram
 *
 * @param float $kg Weight in Kilogram
 */
public function setWeight($kg) {}

This code is easy to document because you need to explain what "kg" means because some people might use a different system and you can't google for "kg".

I'm in favor of writing

class Person { 
/** 
 * @param float $kilogram
 */
public function setWeight($kilogram) {}

The doc block is superfluous because calling setWeight on Person can REALLY be expected to set the Weight on a Person. No need to write that out again.

Using $kilogramm as a parameter also saves you the trouble of explaining it in the docs and I'd say, depending on your environment, everyone can be expected to google for "kilogram" if he really doesn't know the measurement unit.

---

@PHPDoc documentation

• All my humble opinion of course
• If you don't use type-hinting always use @param tags.
• Always use @return tags
• Don't use @author tags at all. Collection code ownership is more valuable and the information is in the source control repository anyways.
• Only use @copyright tags if you have to. I like only having a LICENSE file but I'm not a lawyer so it might be necessary.

Inline comments:

public function generateReport() {
  // get the db connection
  $reg = WhateverGlobalStorage::get(“db“);
  // auth
  if(!$reg->getOne("SELECT view_report FROM USER ...")) {}
  // template
  $id = $reg->getOne("select ... "); 
  // render
  new ReportTemplate($id); // ...
}

If those are separate "blocks" just move them to descriptive named functions

public function generateReport() {
  $this->checkAuthentication();
  $template = this->createReportTemplate();
  $this->renderReport($template);
}
// Not perfect but at least you can grasp what the method does much quicker

Additional resources:

Slides of a presentation I gave on the subject on some conferences: Slideshare: clean-code-stop-wasting-my-time

And additional small, little older, rant: they-told-you-to-document-everything-they-lied

Book references:

Clean Code: A Handbook of Agile Software Craftsmanship

Refactoring: Improving the Design of Existing Code

A longer example

abstract class xyzRequest {
 /**
   * Initializes this xyzRequest.
   *
   * Available options:
   *
   *  * logging: Whether to enable logging or not (false by default)
   *
   * @param  xyzEventDispatcher $dispatcher  An xyzEventDispatcher instance
   * @param  array  $parameters  An associative array of initialization parameters
   * @param  array  $attributes  An associative array of initialization attributes
   * @param  array  $options     An associative array of options
   *
   * @return bool true, if initialization completes successfully, otherwise false
   *
   * @throws <b>xyzInitializationException</b> If an error occurs while initializing this xyzRequest
   */
  public function initialize(xyzEventDispatcher $dispatcher, $parameters = array(), $attributes = array(), $options = array()) {

Lets see, line by line, what that documentation tells you. (I'm joking around here a little bit to get my point across)

* Initializes this xyzRequest.

So calling ->initialize on a xyzRequest initializes that request? Really? Ok then, if you say so!

   * Available options:
   *
   *  * logging: Whether to enable logging or not (false by default)

We are told the options for the third parameter, not for the second or third param but maybe we know those if we know the framework? (Since we are not able to figure out what ->initialize does without someone telling use we might not be that smart...)

   * @param  xyzEventDispatcher $dispatcher  An xyzEventDispatcher instance

Yeah, the type-hint is there. So if the method expects a "xyzEventDispatcher instance" we need to pass in a "xyzEventDispatcher instance". Good to know.

   * @param  array  $parameters  An associative array of initialization parameters
   * @param  array  $attributes  An associative array of initialization attributes
   * @param  array  $options     An associative array of options

Ok. So it's not a linear array. But that I need to pass "initialization parameters" to an "initialize" method I could have figured out.

Still no clue what I actually need to pass in there but as long as its documented it has to be fine!

   * @return bool true, if initialization completes successfully, otherwise false

So a boolean return value is "true" for "good" and "false" for bad".

   * @throws <b>xyzInitializationException</b> If an error occurs while initializing this xyzRequest
   */

So an exception is thrown if an error occurs while we are doing what the function is named?

So exceptions are used for error cases. Ok. Good to know.

• Doesn't tell me the difference between return false and an exception.
• The @throws its self is fine because it adds information
• Btw: Why is this BOLD and not a @link

Personally, I only comment in the constructors if there is something special to comment about it (like a special initialization).

I wouldn't say that's the "most useful" way to go, but it keeps the code neat, and repeating two times the same thing isn't really needed (if that's your concern).