Documentation Quirks

Well-maintained software should have quality API documentation. Certainly. However, just as the absence of documentation is a mistake, so too is its redundancy. Writing documentation comments, much like designing an API or user interface, requires thoughtful consideration.

By thoughtful consideration, I do not mean the process that occurred in the developer's mind when they complemented the constructor with this comment:

class ChildrenIterator
{
	/**
	 * Constructor.
	 *
	 * @param array $data
	 * @return \Zend\Ldap\Node\ChildrenIterator
	 */
	public function __construct(array $data)
	{
		$this->data = $data;
	}

Six lines that add not a single piece of new information. Instead, they contribute to:

• visual noise
• duplication of information
• increased code volume
• potential for errors

The absurdity of the mentioned comment may seem obvious, and I'm glad if it does. Occasionally, I receive pull requests that try to sneak similar rubbish into the code. Some programmers even use editors that automatically clutter the code this way. Ouch.

Or consider another example. Think about whether the comment told you anything that wasn't already clear:

class Zend_Mail_Transport_Smtp extends Zend_Mail_Transport_Abstract
{
	/**
	 * EOL character string used by transport
	 * @var string
	 * @access public
	 */
	public $EOL = "\n";

Except for the @return annotation, the usefulness of this comment can also be questioned:

class Form
{
	/**
	 * Adds group to the form.
	 * @param  string $caption	   optional caption
	 * @param  bool   $setAsCurrent  set this group as current
	 * @return ControlGroup
	 */
	public function addGroup($caption = null, $setAsCurrent = true)

If you use expressive method and parameter names (which you should), and they also have default values or type hints, this comment gives you almost nothing. It should either be reduced to remove information duplication or expanded to include more useful information.

But beware of the opposite extreme, such as novels in phpDoc:

	/**
	 * Performs operations on ACL rules
	 *
	 * The $operation parameter may be either OP_ADD or OP_REMOVE, depending on whether the
	 * user wants to add or remove a rule, respectively:
	 *
	 * OP_ADD specifics:
	 *
	 *	  A rule is added that would allow one or more Roles access to [certain $privileges
	 *	  upon] the specified Resource(s).
	 *
	 * OP_REMOVE specifics:
	 *
	 *	  The rule is removed only in the context of the given Roles, Resources, and privileges.
	 *	  Existing rules to which the remove operation does not apply would remain in the
	 *	  ACL.
	 *
	 * The $type parameter may be either TYPE_ALLOW or TYPE_DENY, depending on whether the
	 * rule is intended to allow or deny permission, respectively.
	 *
	 * The $roles and $resources parameters may be references to, or the string identifiers for,
	 * existing Resources/Roles, or they may be passed as arrays of these - mixing string identifiers
	 * and objects is ok - to indicate the Resources and Roles to which the rule applies. If either
	 * $roles or $resources is null, then the rule applies to all Roles or all Resources, respectively.
	 * Both may be null in order to work with the default rule of the ACL.
	 *
	 * The $privileges parameter may be used to further specify that the rule applies only
	 * to certain privileges upon the Resource(s) in question. This may be specified to be a single
	 * privilege with a string, and multiple privileges may be specified as an array of strings.
	 *
	 * If $assert is provided, then its assert() method must return true in order for
	 * the rule to apply. If $assert is provided with $roles, $resources, and $privileges all
	 * equal to null, then a rule having a type of:
	 *
	 *	  TYPE_ALLOW will imply a type of TYPE_DENY, and
	 *
	 *	  TYPE_DENY will imply a type of TYPE_ALLOW
	 *
	 * when the rule's assertion fails. This is because the ACL needs to provide expected
	 * behavior when an assertion upon the default ACL rule fails.
	 *
	 * @param  string								   $operation
	 * @param  string								   $type
	 * @param  Zend_Acl_Role_Interface|string|array	 $roles
	 * @param  Zend_Acl_Resource_Interface|string|array $resources
	 * @param  string|array							 $privileges
	 * @param  Zend_Acl_Assert_Interface				$assert
	 * @throws Zend_Acl_Exception
	 * @uses   Zend_Acl_Role_Registry::get()
	 * @uses   Zend_Acl::get()
	 * @return Zend_Acl Provides a fluent interface
	 */
	public function setRule($operation, $type, $roles = null, $resources = null, $privileges = null,
							Zend_Acl_Assert_Interface $assert = null)

Generated API documentation is merely a reference guide, not a book to read before sleep. Lengthy descriptions truly do not belong here.

The most popular place for expansive documentation is file headers:

<?php
/**
 * Zend Framework
 *
 * LICENSE
 *
 * This source file is subject to the new BSD license that is bundled
 * with this package in the file LICENSE.txt.
 * It is also available through the world-wide-web at this URL:
 * http://framework.zend.com/license/new-bsd
 * If you did not receive a copy of the license and are unable to
 * obtain it through the world-wide-web, please send an email
 * to license@zend.com so we can send you a copy immediately.
 *
 * @category   Zend
 * @package	Zend_Db
 * @subpackage Adapter
 * @copyright  Copyright (c) 2005-2012 Zend Technologies USA Inc. (http://www.zend.com)
 * @license	http://framework.zend.com/license/new-bsd	 New BSD License
 * @version	$Id: Abstract.php 25229 2013-01-18 08:17:21Z frosch $
 */

Sometimes it seems the intention is to stretch the header so long that upon opening the file, the code itself is not visible. What's the use of a 10-line information about the New BSD license, which contains key announcements like its availability in the LICENSE.txt file, accessible via the world-wide-web, and if you happen to lack modern innovations like a so-called web browser, you should send an email to license@zend.com, and they will send it to you immediately? Furthermore, it's redundantly repeated 4,400 times. I tried sending a request, but the response did not come 🙂

Also, including the copyright year in copyrights leads to a passion for making commits like update copyright year to 2014, which changes all files, complicating version comparison.

Is it really necessary to include copyright in every file? From a legal perspective, it is not required, but if open source licenses allow users to use parts of the code while retaining copyrights, it is appropriate to include them. It's also useful to state in each file which product it originates from, helping people navigate when they encounter it individually. A good example is:

/**
 * Zend Framework (http://framework.zend.com/)
 *
 * @link	  http://github.com/zendframework/zf2 for the canonical source repository
 * @copyright Copyright (c) 2005-2014 Zend Technologies USA Inc. (http://www.zend.com)
 * @license   http://framework.zend.com/license/new-bsd New BSD License
 */

Please think carefully about each line and whether it truly benefits the user. If not, it's rubbish that doesn't belong in the code.

(Please, commentators, do not perceive this article as a battle of frameworks; it definitely is not.)