Difference between revisions of "Development/Coding standard"

From AtoM wiki
(Remove editorial style guide section (was broken links and internal notes copied over from Qubit))
(Constructors)
Line 97: Line 97:
 
== Constructors ==
 
== Constructors ==
  
Don't use empty empty constructor argument lists, since they're not required,
+
Don't use empty constructor argument lists, since they're not required,
  
 
<pre>
 
<pre>

Revision as of 13:32, 15 July 2015

Main Page > Development > Development/Coding standard

On this page you will find an overview of some of the coding practices we follow when developing AtoM, and which we look for as part of our Code review process. The AtoM project is based on the symfony (1.4) framework, which is written in PHP. We've adopted the following standards, in addition to symfony's coding standard and best practices for PHP development.

In general, we recommend that anyone wanting to work with AtoM code familiarize themselves with git, the PHP Symfony 1.x framework, Twitter Bootstrap (2.3), and Elasticsearch. You should also be comfortable working in the command-line! Below are a few basic resources to get you started:

Variable names

We prefer, whenever possible, using camelCase variables:

$myVariable = 5; // Preferred
$my_variable = 5; // Not preferred

There are a lot of variations in what is considered to be camelCase - we prefer starting with a lower-case letter.

Braces

Always use braces, even when not necessary:

if ($x) // Preferred
{
  myFunc();
}

if ($x) // Not preferred
  myFunc();

Also, always ensure braces are on their own lines:

if ($x) // Preferred
{
  //...
}

if ($x) { // Not preferred
  // ...
}

PHP open tags

Never use PHP short open tag (<?); instead, please use the full tag <?php instead.

To ensure you remember this during development, you can disable short open tags support in your PHP environment (short_open_tag directive in your php.ini file).

count() vs. sizeof()

Always use count(). Also see below for using count() to test for empty arrays.

empty()

Never use empty() for checking whether an array value is empty. Instead always use 0 < count(). This is because our arrays may be objects implementing the ArrayAccess interface, in which case empty() may not behave as expected.

See: http://www.symfony-project.org/book/dev/07-Inside-the-View-Layer#Escaping%20Arrays%20and%20Objects

An advantage of using count() > 0 over empty() is that empty() cannot be called using variable functions.

See: http://php.net/empty

empty() is a language construct and doing so is a syntax error.

Templates

Don't use trailing semicolons in templates unless required. Generally the trailing semicolon is implied by the PHP close tag. The trailing semicolon is only required after alternative PHP syntax end control statements (endif, endforeach, endwhile, etc.)

<?php echo $someVar ?> // Good, semicolon is implied by the PHP close tag
<?php echo $someVar; ?> // Unnecessary
<?php endif; ?> // Necessary

Constructors

Don't use empty constructor argument lists, since they're not required,

$foo = new Foo; // Good, no argument list
$foo = new Foo(); // Unnecessary
$foo = new Foo($bar, ...); // Necessary

When it is useful to have a constructor method, either because you can chain methods but not constructors (create...()->aaa()->bbb() works, but new Foo->aaa()->bbb() does not), or in order to support constructor overloading (createFromFoo($foo) and createFromBar($bar)), or because the actual constructor must have protected visibility, the constructor method should be named like create...(). It shouldn't be named like new...() because new is a reserved word in PHP.

Return statements

As per symfony standard, add an empty line before any return statements *except* where it would leave an empty line after an opening curly brace (e.g. a one line function). Empty lines should *never* follow an open curly brace or precede a close curly brace

Whitespace

Use one empty line between functions.

is_null()

Unlike the symfony coding standard, use null === $foo instead of is_null($foo). This is because there's no equivalent to is_null() for "false === $foo" or "0 === $foo", and we prefer to use consistent syntax for "null === $foo", "false === $foo", and "0 === $foo"

symfony has since adopted this convention, in symfony >= 1.3

$options

Instead of functions with long lists of optional arguments, we prefer passing these arguments in an $options array,

public function foo($required1, $required2, array $options = array()) // Preferred
public function foo($required1, $required2, $option1 = null, $option2 = null, ...) // Not preferred

This saves having to pass several null arguments when you want to specify only the last option, which is prone to getting the number of arguments in the function signature wrong, and accidentally specifying the wrong option

It's interesting how symfony mixes associative and ordered options/parameters; see for example: sfLogger::listenToEvent()

In the function body, set default values for optional arguments with array addition,

$options += array('option1' => 'foo', 'option2' => 'bar');

Links

Don't use symfony internal URIs. Instead, always use an array of parameters for generating URIs. This is because it's easy to confuse internal and external URIs, and internal URIs add yet another URI format to remember

link_to('link text', array('module' => user', 'action' => 'login')); // Good
link_to('link text', 'user/login'); // Bad
link_to('link text', '/login/'); // Incorrect

substr()

Use substr() to check if a string begins or ends with another string. It's possible to check if a string begins with another string with strncmp(), but strncmp() isn't able to check if a string ends with another string and we prefer to use a consistent pattern for both. Also, the return value ofstrncmp() (< 0 if str1 is less than str2 ; > 0 if str1 is greater than str2 , and 0 if they are equal) is confusing.

if (substr($string, 5) == 'HTTP_') // Good
if (substr($string, -8) == '_wrapper') // Good
if (!strncmp($string, 'HTTP_', 5)) // Awkward
if (!strncmp($string, '_wrapper', -8)) // Not supported

javascript

Never use the JavaScript pseudo protocol. Always use standard JavaScript events for triggering behavior. Because the "a" tag must have an "href" attribute, use href="#". This is standard practice in YUI (see the tree view) and symfony (see the debug toolbar). A possible advantage of href="#" is that it could possibly be used to support the back button.

<!-- Good -->
<a class="foo" href="#">
[...]
$('.foo').click(foo);

<!-- Bad -->
<a href="javascript:foo()">