COmanage Coding Style

Background

A common coding style ensures consistency of the look and feel of the code, regardless of who wrote a given portion. This, in turn, increases readability, makes the code easier to understand, and makes the project look more polished and professional.

Many of the style decisions here are arbitrary. The purpose of this document is not to answer the general question "What is the proper number of spaces to indent code?" but the specific question "What number of spaces is code indented in the COmanage codebase?" (The answer is 2, described below.)

For examples of other coding style guides, see The Zend Framework (PHP), Apache (C), and Google (various).

General

• Use the most compact notation that is still easy to understand.
• Follow CakePHP guidelines, such as for file names.
• If there's a reason to have an exception to these guidelines in the code, then there's a reason to update this document.

SVN Commit and JIRA Logs

• SVN commit logs must include the JIRA issue they are addressing. In addition, a useful description must be included, so that svn log will show useful messages.
• JIRA comments must include a brief explanation of what was fixed, to facilitate historical searches. (Simple fixes, such as typos, need no explanation.) JIRA issues must be updated with corresponding SVN commit revision numbers.

Error Reporting

There are a number of mechanisms for reporting errors. Which to use depends on context.

setFlash

For most interactive transactions, use $this->Session->setFlash to set a message that will be rendered with the next view. This includes standard pages ("default" layout) as well as page sections retrieved via AJAX calls ("ajax" layout).

REST HTTP Response Code

For API transactions (ie: XML or JSON over HTTP), a suitable message and HTTP Response Code should be generated, as per the API definition.

Exceptions

Exceptions should be used rarely, and only in situations where the application cannot recover gracefully enough to generate an error using a preferred method.

CakeError

CakeErrors are deprecated as of 2.0 and should not be used.

Debugging

Debugging should be done with debug() rather than print. Debugging should not be committed, and this will make it easier to find stray debug statements before committing. Furthermore, debug() statements will become no-ops in a production setup if they do accidentally end up in the codebase.

The use of $this->log() is also acceptable, although this is subject to revision as part of CO-96 or the implementation of other logging features.

Making Breaking Changes

Breaking changes cause version number changes as defined by Semantic Versioning.

As of v1.0, for changes that require updates to existing data, such changes must be scripted and run as part of the upgrade shell.

Naming Correct Case

Methods

Camel case with lower initial:

$this->bindModel();
$this->initializeParentCou();

Methods should be in alphabetical order within the file that contains them. eg: add should be before delete in FooController.php.

Variables

Camel case with lower initial:

$cou = 'whatever';
$couAllowed = array();
$isInCou = false;

Avoid very short names except in very compact contexts, such as for loops.

View Variables

View Variables are those set in a Controller and passed to a View. View Variables must be prefixed vv_.

Controller:

$this->set('vv_my_variable', $this->Model->find('first'));

View:

print $vv_my_variable

Quotes

Single Quotes

Single quotes are used when using a string as an index.

$foo['this']['that'];

Double Quotes

Double quotes are used when quoting a string in other contexts.

$txt = "This is some text.";

Whitespace

Indentation

Indentation is in increments of two (2) spaces. Tabs are not used.

function foo($i) {
  $j = $i + 1;

  if($j > 1)
    bar($j);
}

Separation of Characters

In general, whitespace is omitted where it is not necessary. For example:

function foo($a);

$a = $b['foo'];

And not:

function foo ( $a );

$a = $b['foo'];

The exceptions are where extra whitespace dramatically increases readability:

// Argument lists, arrays, etc
passingArguments($first, $second, $third);

// Nested array references
$a = $b[ $c['foo'] ];

// If-then-else shorthand
return($a ? $a : $b);

Separation of Statements

In general, all statements, procedures, functions, etc are separated by a blank line. However, "like" statements (such as blocks of variable declarations or calculations performed as part of an operation) are grouped together without an intermediate blank line.

part of previous function;
  end of previous function;

  return(foo);
}

function textFunction() {
  $var = 0;
  $othervar = 1;

  beginning of next function;

End of File

One or more newlines at the end of a file may cause errors (the page does not render) on some systems, including MAMP and Ubuntu, and so must be avoided.

See also #NoClosingTag.

Alignment

Curly Braces

Curly braces ({,}) start on the same line as the introductory statement, separated by a space. Continuation statements (such as else) begin on the same line as the previous block's closing brace, again separated by a space.

Curly braces may be omitted only when each contained clause is one line, however in general they should be used.

OK:

if($a > 0) {
  return(true);
} elseif($b > 0) {
  return(false);
}

if($a > 0)
  return true;
elseif($b > 0)
  return false;

Not OK:

if($a > 0) {
  $a++;
  $foo = bar;
} else
  return false;

Parentheses

Parentheses are used even when considered optional. The exception is return statements, since return is a language construct and not a function.

OK:

print ($a ? $a : $b);

return false;

Not:

print $a ? $a : $b;

return(false);

When creating a list within parentheses, such as when constructing an array or passing parameters to a function, if the entire list is not readable on one line, indent one level and begin on the next line:

$v = array('small');
$w = array(
  "the beginning of a longer list",
  "the continuation of a longer list",
  "the conclusion of a longer list"
);

Hashes and Arrays

When constructing a hash, align on the => as much as possible:

$h = array(
  'A'  => "Option A",
  'B'  => "Option B",
  'YZ' => "Option YZ"
);

When constructing an array, align on the = as much as possible:

$args['foo']['bar']        = "Sushi";
$args['foo']['restaurant'] = "Hibachi";

Conditionals

if statements with complex conditionals should have each conditional on a new line, aligned with the previous conditional. Nested conditionals should be further indented.  Exceptions are made for those that are easy to read and fit on one line.

if(($a == $b)
   || (($a == $c)
       && ($d == $e))

Comments

Comment Delimiters

Comments are exclusively delimited with double slashes (//). The exceptions are the docblock comments, and that code may be temporarily commented with C-style comments (/* */).

Top of File

The top of each file must include the following header in docblock format:

/**
 * COmanage Directory People Controller
 *
 * Copyright (C) 2012 University Corporation for Advanced Internet Development, Inc.
 *
 * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with
 * the License. You may obtain a copy of the License at
 *
 * http://www.apache.org/licenses/LICENSE-2.0
 *
 * Unless required by applicable law or agreed to in writing, software distributed under
 * the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
 * KIND, either express or implied. See the License for the specific language governing
 * permissions and limitations under the License.
 *
 * @copyright     Copyright (C) 2012 University Corporation for Advanced Internet Development, Inc.
 * @link          http://www.internet2.edu/comanage COmanage Project
 * @package       directory
 * @since         COmanage Directory v0.1
 * @license       Apache License, Version 2.0 (http://www.apache.org/licenses/LICENSE-2.0)
 * @version       $Id$
 */

Use "Registry" or "Directory" as appropriate.

@package is either directory or registry.

Note that $Id is an SVN keyword that must be set on a per-file basis. (It will not be updated automatically otherwise.) This can be done with

svn propset svn:keywords "Id" foo.php

Don't add a new file with the Id string copy and pasted from a different file.

Function Descriptions

Functions must have a docblock of the following form preceding the function definition:

/**
   * Update the amount of foo
   * - precondition: $this->foo set
   * - postcondition: Flash message set on error
   *
   * @since  COmanage Directory 0.1
   * @todo   Handle nested foo
   * @param  integer $i New amount of foo
   * @param  string  $s New name for foo
   * @return void
   * @throws RuntimeException
   */

  public function foobar($i,
                         $s) {

Comments and descriptions are written in normal English, except periods are omitted if there is only one sentence.

precondition, postcondition, todo, param, and throws are optional, and should be omitted if no relevant information applies.

Inline Comments

Comments should be included where it isn't patently obvious what is going on. Comments should be written in normal English using normal grammar and syntax.

// We need to figure out what the person's name is as asserted by
  // the home organization, so pull it from the Org Identity.

When a code block relates to a detailed JIRA issue (eg: a bug or a specific change), link to the issue in the comment. Do not link to JIRA for big feature requests (those that encompass more than a small block of code).

// This change is for CO-90210. Don't undo it!
  // https://bugs.internet2.edu/jira/browse/CO-90210

Data Filtering

Input and output sanitization should be achieved using standard PHP filters.  Cake's native Sanitize:: filter has been deprecated as of Cake 3 and should be avoided.  Guidelines for converting existing Cake Sanitizate:: filters to PHP filters is documented in CO-667.

PHP-isms

No Short Tags

The full PHP tag must be used.

<?php some stuff; ?>

No Closing Tag

To avoid errors related to whitespace at the end of file, do not close PHP files with ?> if they are primarily PHP code.

print, not echo

echo is a language construct, not a function, and so it's use within COmanage is deprecated to avoid unexpected behavior. (print is also a language construct, but behaves like a function.)

Scoping

All object-oriented methods and variables must be appropriately and explicitly scoped (private, public, etc).

Controls Split Across Tags

When splitting control structures across multple <?php ?> tags (ie: to intersperse HTML), use colon notation with comments in closing tags.

Preferred:

<?php if($foo): ?>
  stuff;
  <?php if($bar): ?>
    morestuff;
  <?php endif; // bar ?>
<?php endif; // foo ?>

Not Preferred:

<?php if($foo) { ?>
  stuff;
<?php } ?>

CakePHP-isms

Arrays as Arguments

Where an array is required as an argument (used very commonly in CakePHP), define the array first and then pass it.

Preferred:

$args = array();
$args['fields'][] = "MAX(ordr)+1 as m";
$args['order'][] = "m";

$o = $this->CoEnrollmentAttribute->find('first', $args);

Not Preferred:

$o = $this->CoEnrollmentAttribute->find('first',
                                        array('fields' =>
                                              array("MAX(ordr)+1 as m")),
                                        array('order' =>
                                              array("m")));

SQL Query Optimization

In general, use Containable Behavior to constrain what Cake is pulling from the database. Cake will usually try to pull a bunch of associated data, which may or may not match what you actually need in a given context. You can use Containable to specify exactly which associated data you want returned.

Don't return anything but CoPerson:

$this->CoPerson->contain();

Obtain OrgIdentity and Name:

$this->OrgIdentity->contain('Name');

Obtain OrgIdentity, Name, CO, and CO Groups the Org Identity is a member of:

$args['contain'][] = 'Name';
$args['contain']['CoOrgIdentityLink']['CoPerson'][0] = 'Co';
$args['contain']['CoOrgIdentityLink']['CoPerson']['CoGroupMember'] = 'CoGroup';
$orgIdentities = $this->OrgIdentity->find('all', $args);

URLs (API)

URLs are specified in the REST API. The general form is

/controller/id.format

where format is the requested response format. The action is specified by the HTTP verb:

• GET: index (if no id), view
• POST: add
• PUT: edit

URLs (Browser)

GET operations follow the form

/controller/action/arguments

as in the example

/co_people/index/co_id:2

arguments should map to their database name (ie: underscored inflection) whenever possible.

POST operations follow the form

/controller/action

as in the example

/co_people/add

Arguments are embedded in the POST body. This facilitates the scenario where a form submission errors out (perhaps the user left out a required field) and Cake regenerates form with a URL having no args.

GET operations that are intended to convert to a POST operation (ie: an add or an edit that renders a form via GET and then POSTs the form) must have the URL arguments converted to hidden variables, usually by the form generation in the corresponding fields.inc.

Use Containable, not Recursive

$this->recursive, in addition to being too blunt an instrument to determine how much associated data is retrieved with a model (it generates way too many SQL queries in most circumstances), also doesn't currently work correctly with the Grouper datasource (CO-268).

Use of Linkable Behavior is also acceptable, though it should be considered somewhat experimental.

Magic Finds

Do not use the magic findByX() functions, as they do not appear to trigger Behaviors (especially ChangelogBehavior).

updateAll()

Do not use updateAll(), as it does not trigger Behaviors (especially ChangelogBehavior).