Actions

Difference between revisions of "Developer Area/Coding guidelines"

From Mahara Wiki

< Developer Area
Line 31: Line 31:
 
* Avoid editing whitespace unrelated to your specific code change, when submitting a patch. It makes reviewing the patch slightly more difficult.
 
* Avoid editing whitespace unrelated to your specific code change, when submitting a patch. It makes reviewing the patch slightly more difficult.
 
** In git, the "-w" flag will ignore whitespace changes
 
** In git, the "-w" flag will ignore whitespace changes
** Our version of gerrit (2.5) doesn't have an "ignore whitespace" setting, unfortunately.
+
** Our version of gerrit (2.6) can also be configured to ignore whitespace when reviewing changes
  
 
=Comments=
 
=Comments=

Revision as of 17:27, 30 July 2013

This page documents the coding conventions used by Mahara. Everything that ends up in the git repository (other than third-party code) should follow these standards.

You can also have a look at the Reviewer's manual to see what reviewers will look at when looking at code submissions.

PHP Files

JavaScript Files (or snippets)

CSS Files

SQL snippets

XML Files

TPL Files (may change in future if move away from smarty, ie to twig)

File Headers

All files must adhere to the BasicPHPFileTemplates. There are slightly different templates depending on whether the script is to be included or whether it is to be directly hit.

Whitespace

Required:

  • Indentation is using four (4) spaces, not tabs.
  • Line endings should be "Unix-style" (\n), not "Windows-style" (\r\n)

Recommended:

  • There is no requirement that the scripts are wrapped at 80 columns, however developers are asked to wrap long lines sensibly.
  • Avoid extra whitespace at the end of lines (it causes warnings in many code sniffers)
    • Although, the only real harm it causes is that you can see it when you tell your editor to let you see whitespace
  • Avoid editing whitespace unrelated to your specific code change, when submitting a patch. It makes reviewing the patch slightly more difficult.
    • In git, the "-w" flag will ignore whitespace changes
    • Our version of gerrit (2.6) can also be configured to ignore whitespace when reviewing changes

Comments

  • Comments at the head of files, functions, classes, etc should use the /** ... */ comment form for compatibility with PHPDoc.
  • Explanatory comments in the body of the code should use only // comment markers, regardless of the length of the comment.
    • This allows /* ... */ markers to be used for quick mass commenting during debugging.
  • Comments about a line of code should be placed on their own line above that line of code, rather than at the end of it.
    • This makes them more visible to people scanning the page, and ensures they won't be hidden off the side of the page if the reader's editor window is small.

Example:

/**
 * This is an explanation of what this function does.
 *
 * @param boolean $bar description of the purpose of $bar
 * @param string $baz description of the purpose of $baz
 * @param array $qux description of the purpose of $qux
 * @return array|string This function either returns $qux with $baz added to it, or $baz with $bar added to it
 */
function foo($bar, $baz='bax', $qux=array()) {
    
    // This part is difficult and requires a long
    // explanation. This is that explanation. I am
    // still typing to see if I can make it to four
    // lines.
    if ($bar) {
        
        // put it on the end
        $qux[] = $baz;
        return $qux;
    } else {
        
        // I'm sure this makes sense somehow
        return $baz || ((string) $bar);
    }
}

Language Constructs

Strings

Recommended:

  • Strings should be quoted with single quotes (')
    • Unless you are interpolating a variable
    • Or writing a string with lots of single quotes in it.
  • Variable interpolations should be written with {curly braces} around them.

Examples:

$a = 'This is a string.';
$b = "This's a string that's got lots of apostrophes (i.e. ' characters), in it. Ain't it pretty nifty?";
$c = "My {$field} is {$users[$1]->$field}";

require/require_once/include/include_once

These statements should have brackets around the argument. There is no space between the statement and the bracket. In effect, this is the same as a function call.

require_once('my/file.php');

NOT:

require_once 'my/file.php';
require_once (
'my/file.php');

Plugin files must be included using the File:Http.pngsafe_require function. This function ensures that there are no vulnerabilities in accessing a plugin file. Plugins that do not use this function to include files should be considered vulnerable to a possible security vulnerability.

Any include statements in the core should never have a variable in their name, to reduce the chance of an arbitary include vulnerability.

The include path is set automatically to be the current directory plus the lib directory - there is no need to specify an absolute directory path.

if/while/for statements

There is one space after each language construct, and no spaces between brackets or parentheses and the arguments inside them, unless what is inside them becomes too long for one line, in which case a special format will be used.

Examples of what is good:

if (1 == 2) {
    die(
'omg');
}

while (
$i < 3) {
    echo
'I am ' . $i;
}

for (
$i = 0, $j = 7; $i < 34, $j != 3; $i++, $j--) {
    echo (
$i * $j) . ' is a silly number';
}

// Note bracing style of else if/else clauses
// Also note: do not use elseif
if ($a == $b) {
    echo
"a = b";
}
else if (
$c == $d) {
    echo
"c = d";
}
else {
    echo
"e = ?";
}

// Ternary operator
$a = ($b == true) ? 'hello' : 'goodbye';
// When used with other things on each side in the same statement:
$a = 'foo' . (($b == true) ? 'hello' : 'goodbye') . 'bar';

// A complicated condition. Note location of condition,
// and location of ending bracket and brace.
// todo: what was the decision here regarding conditions on the "if (" line?
if (
    
$sacrificedgoats == $availablegoats
    
&& ($dogs == $cats
    
|| 1 != 2)
    || (
$yesterday >> $tomorrow == 4)
) {
    
// Do stuff
}

Examples of what is bad:

if ( $a == $b )
{
    echo
'hi';
}

if(
$c == $d) {
    echo
'hi';
} else {
    echo
'bye';
}

if (
$e==$f)
{
    echo
'hi';
}
else
{
    echo
'bye';
}

What should be never used:

// For a really short condition
if ($a == $b) $c = 1;

Variables and Arrays

Variables should be named with no underscores, and no capital letters. This policy may become more lenient in future if variable names are discovered that are not easily readable without underscores.

Do not name variables as negatives where possible - always use positive names and logical inversion (!) where required.

Hungarian notation is forbidden.

Aligning = signs for variable assignment is up to the developer, although they should use common sense in such decisions.

Good:

$found = false;
$ponies = 0;
$servername = $overkill;

// Only in relation to controlling for/while loops
for ($i = 10; $i > 5; $i--) {
    echo
$i;
}

// Aligning = signs
$mine   = 0;
$yours  = 0;
$theirs = 0;

// Only if the number of elements to initialise is small.
$array = array(1 => 'hello');

// If large number of elements to initialise:
$array = array(
    
1  => 'hello',
    
2  => 'goodbye',
    
10 => 'erm'
);

Bad:

// Negative name
$notfound = true;
// Underscores
$my_variable = '';
// Hungarian
$count_i = 6;
// Uppercase letters
$myFish = $yourFish;

$array = array(
                
1 => 'hello',
                
2 => 'goodbye',
                
10 => 'erm'
              
);

Binary Operators

Always have a space on each side:

echo 1 + 2;
if (
1 == 2)
echo
'hello' . ' world';

|| and && will always be used for 'or' and 'and' respectively. Resolve priority collisions with extra parentheses as required.

Classes, Functions and Methods

Nameing

Classes should start with upper case character, then each word capitalized.

ExampleWhizbang

Functions should not use camel case, and instead use underscores as word seperators

do_something()

NOT

doSomething()

The built-in PHP class "stdClass" should be written with a lowercase s, uppercase C, because that is its actual name. Other capitalizations, such as "StdClass" will not cause an error because invocations of classnames and functions are case-insensitive in PHP. However, it is good style to use the same capitalization as the declaration of the class/function.

Constructors without arguments should be called with empty parentheses after them.

Good:

$a = new stdClass();
$b = new RssBlock();

Bad:

// stdClass written incorrectly
$a = new StdClass();

// no parentheses after constructor
$b = new RssBlock;

// lowercase user-created class
$c = new fancyblock();

Declaration:

/**
* PHPDoc comment describing the function
*
* Note how the default value has no spaces
*/
function foo($a, Class $b, $c='') {
    
// source
    
return $value;
}

class
FooBar extends Bar {

    
/**
     * PHPDoc for the field
     *
     * For this project, do not use doThis.
     */
    
public function do_this($c, $d) {
        
// source
    
}

}

Calling:

$result = foo($bar, $baz);
$mine = $object->method($a, $b);

Javascript

All files must adhere to the BasicJSFileTemplates. Javascript files should roughly follow Crockford's code conventions.