Coding conventions (draft)

To a great extend complies with the phpBB Coding Guidelines.

Always use <?php ?>. Others may not work depending on the server config.

<?php
echo 'foo';
?>

Use PascalCasing for classes and functions, camelCasing for variables. Private member variables of classes should use Pascal Casing, prefixed with “m_”.

Variable names should be descriptive, so that you can know or at least guess what the variable contains and is used for.

class Template;
class AdminNotifications;
class Url;
 
$taskId;
$userName;
 
GetTaskDetails();

Length for all should be less than 25 chars.

For loops (and only for loops) you should use single letter variable names. When the depth increases, the variable name increases as well (starting with $i).

for ($i = 0; $i < 10; $i++) {
    for ($j = 0; $j < $array_count; $j++) {
        doFoo();
    }
}

Whenever possible, please avoid them. They can have a huge negative impact on the readability.

// Negative example, taken from Flyspray 0.9.8 (try to understand what this code does ^^)
 
function cryptic_function($lang, $module)
{
  $before = get_defined_vars();
  require_once("lang/en/$module.php");
  $after_en = get_defined_vars();
  $new_var = array_keys(array_diff($after_en, $before));
  $new_var_name = @$new_var[1];
  $new_var['en'] = @$$new_var_name;
  if (file_exists("lang/$lang/$module.php"))
  {
     require_once("lang/$lang/$module.php");
  }
   $new_var[$lang] = @$$new_var_name;
   $$new_var_name = @array_merge($new_var['en'], $new_var[$lang]);
}

Do not use function aliases. So use implode instead of join and count instead of sizeof (for example).

Whenever you create a new class or function, please provide PHPDoc comments like this one:

/**
 * Generates a message depending on the $type and using $data
 * @param integer $type
 * @param array $data usually contains task_id => $id
 * @return array which is array($subject, $body)
 * @access public
 */
function generate_message($type, $data = array())

Constant names should only have uppercase letters, while multiple words are seperated by an underscore.

define('FS_VERSION', '0.9');

If you do multiple, similar assignments, you should align them. The ”&” token should be adjacent to the ”=”, not the variable name.

$user_id    =& $id;
$user_name  =  null;
$user_type  =  ADMIN;

Never omit curly braces. This makes it easier to understand the code. Within each block indent using four (4) spaces. Tabs are only allowed for CSS or Javascript files ¹⁾.

//bad
if ($do) DoStuff();
 
if ($do)
    DoStuff();
 
while ($do)
   DoStuff();
 
// good
if ($do) {
    DoStuff();
}
 
while ($do) {
    DoStuff();
}

You can either put braces in their own line, or put the opening brace in the same line as the statement. For switch and classes always put braces in their own line. Single line statements should not be used.

// bad
if ($do) {
   DoStuff();}
 
if ($do) {DoStuff();}
 
function DoStuff() {
    die();
}
 
// good
if ($do) {
    DoStuff();
}
 
if ($do)
{
    DoStuff();
}
 
function DoStuff()
{
    die();
}

Always leave one space between tokens (if while . ? > , < = ; & || …), similar to the written language. Only use lowercase letters for keywords like if, true, null etc.

$i=0; // bad
$i = 0;  // good
 
if($i<7) ... // bad
if ($i < 7) ... // good
 
if ( ($i < 7)&&($j > 8) ) ... // bad
if ($i < 7 && $j > 8) ... // good
 
DoStuff( $i, 'foo', $b ); // bad
DoStuff($i, 'foo', $b); // good
 
for($i=0; $i<$size; $i++) ... // bad
for ($i = 0; $i < $size; $i++) ...  // good
 
$i=($j < $size)?0:1; // bad
$i = ( ($j < $size) ? 0 : 1 ); // good

Always use /* */ for multi line and // for single line comments. You can also use comment “artwork” to divide a script visually.

/* This is a
   multi line comment */
 
/**********************\
*  Next example        *
\**********************/
 
// Single line

If you call a function, do not put a space between the function name and the opening parenthesis.

doFoo();

If you call a method, do not use ”→” but only ”::” if the method is a static method (or can be used as such).

// In Flyspray for example:
Flyspray::Redirect(...);
 
$user->can_DoStuff();

If a statement can be used both with and without parentheses (like require), do not use parentheses.

require 'myfile.php';
include 'another_file';
 
if ($do) {
    doFoo();
}
 
echo 'bar';

Do you know the exact precedence of all the operators in PHP? Neither do I. Don't guess. Always make it obvious by using parentheses to force the precedence of an equation so you know what it does. Remember to not over-use this, as it may harden the readability. Basically, do not enclose single expressions. Examples:

// what's the result? who knows.
$bool = ($i < 7 && $j > 8 || $k == 4);
 
// now you can be certain what I'm doing here.
$bool = (($i < 7) && (($j < 8) || ($k == 4)));
 
// But this one is even better, because it is easier on the eye but the intention is preserved
$bool = ($i < 7 && ($j < 8 || $k == 4));

Also use || instead of OR and && instead of AND. Use != instead of <> .

To increase the readability, increment/decrement operators should always be used on their own lne.

// bad
$array[++$i] = $j;
$array[$i++] = $k;
 
// good
$i++;
$array[$i] = $j;
$array[$i] = $k;
$i++;

Use echo.

echo 'foo';

In case you want:

echo 'foo' . $a . '..' . $b . '/' ; //.. many more

use printf().

printf('foo%s..%s/',$a, $b); //...many more

or vprintf() in more complicated cases:

vprintf('foo%s..%s/', array_map('htmlspecialchars',array($a,$b)));

instead of:

echo 'foo' . htmlspecialchars($a) . '..' . htmlspecialchars($b) . '/';

what is faster does not matter, code readability is more important.

If you specify a string in double quotes, variables are parsed within it which is slightly more work for the parser. Additional to that there are more escape sequences. So use single quotes as long as it does not contain any variables.

// bad
$foo = "bar";
 
// good
DoStuff($myvar. 'foobar');
DoStuff("{$myvar}foobar");
 
// for many variables, prefer the second example
DoStuff("{$a}{$b}f{$c}oobar");
// or consider sprintf()
$var = sprintf('%s:foo %s - %s', $a, $b, $c);

In PHP you can specify strings without quotes if they don't contain chars which are not allowed in variable names. However, since this is bad style and generates a PHP notice (in fact an unquoted variable name should be a constant) we won't do it. Never.

// bad
$foo = $assoc_array[blah];
$foo = bar;
 
// good
$foo = $assoc_array['blah'];
$foo = 'bar';

Only use it for simple expressions, preferably for assignments. Also add parentheses around the whole expression.

// bad
($i < $size && $j > $size) ? DoStuff($foo) : DoStuff($bar);
 
// good
$val = ( ($foo) ? $foo : $bar );

Do not use empty(), since this can only be applied to variables. You can not use it for expressions:

// doesn't work
if (empty(Get::val('foo'))) ...

It is recommended to make use of type juggling here:

if ($foo) {
    DoStuff();
}
 
if (Get::val('foo')) {
    DoStuff();
}

Keep in mind that “0” is considered an empty string in this case. If this is not appropriate in your situation use != '' to check if a string is empty.

Do not use magic numbers (numbers that have a certain meaning). Instead we should use constants which makes the code more readable and maintainable. All constants should be saved in a single file though (preferably constants.inc.php).

//bad
if ($type == 1) {
    DoStuff();
} elseif ($type == 2) {
    doFoo();
}
 
// good
define('COMMENT_ADDED', 1);
define('COMMENT_EDITED', 2);
 
if ($type == COMMENT_ADDED) {
    DoStuff();
} elseif ($type == COMMENT_EDITED) {
    DoFoo();
}

Our code should not generate any errors/warnings/notices if E_ALL is set for error reporting. So don't use uninitialized variables, non-extistent array keys or unquoted strings.

Try not to use an unnecessary high level of nesting by making use of continue, break (loops) and return (functions).

// bad
foreach ($arr as $key => $val) {
    if ($key == 1) {
        if ($val == 'foo') {
             echo 'foo';
        } else {
             echo 'val';
        }
        // and much more code
    }
}
 
// good
foreach ($arr as $key => $val) {
    if ($key != 1) {
        continue;
    }
 
    if ($val == 'foo') {
       echo 'foo';
    } else {
       echo 'val';
    }
    // and much more code
}

The curly braces are on their own line. case is indented, everything after case is indented again. Successive case keywords are each on their own line (so when acting like OR). If there is no break at the end of a piece of code after case, add a comment so that it is obvious. Add an empty line after each case block. Example:

switch ($field)
{
    case 'item_summary':
    case 'attached_to_project':
    case 'task_type':
        if ($field == 'task_priority') {
            $old_value = $priority_list[$old_value];
        }
        break;
 
    case 'foo':
        echo 'bar';
        // FALL THROUGH
 
    case TASK_OPENED:
        $return .= $details_text['taskopened'];
        break;
}

Basically files consist of an extension (only “php”) and a descriptive name.
Files which contain a class should use the following sheme: class.classname.php. classname is the (if necessary) abbreviated name of the class. Other include files should use name.inc.php. Examples:

class.tpl.php, fix.inc.php, modify.php

Template files have the extension “tpl” and can contain multiple words seperated by a dot to classify them. Examples:

details.tabs.comment.tpl, common.list.tpl, index.tpl

Use uppercase letters for keywords in SQL statements (like SELECT). Align the query by keywords and only use single line queries if they are very simple.

Always use prepared statements if the query contains PHP variables, it does not matter where they come nor if you “think” it 's safe to use the variable on the query directly, contributions not following this rule will be rejected.

$history = $db->Query('SELECT * FROM {history}');
 
// Check for cached version
$cached = $db->Query("SELECT content, last_updated
                        FROM {cache}
                       WHERE topic = ? AND type = ?",
                      array($task_details['task_id'], 'task'));

for a larger/real world example, see script/reports.php in our svn repository.

All columns and table names are written lowercase everywhere. Column names should consist of two words, seperated by an underscore. Do not use SQL keywords for column names (so that backticks are not needed).

Please try to keep your code reasonable. Thus don't do stuff like the code below. BTW, I did not make this code up. I've seen it myself.

// Define the class [oh really? please spare those comments]
$mail = new PHPMailer();
 
$date = time(); // so why not use $time? time !== date
$time = date('U'); // well... see above
 
function foo() {
    unset($incoming); // unset non existent variables??
    $incoming = ...
}

Do not introduce new code using the following PHP functions/statements:

• eval(), we are attempting to remove existing code that use it, new code that uses eval() will not be accepted.

• Do not use $_SERVER['PHP_SELF'] when you really mean $_SERVER['SCRIPT_NAME'];

• the preg_replace ”/e” modifier should not be used, use preg_replace_callback instead.

• avoid system(), exec(), shell_exec(), popen(), and passthru() calls. If your really need this, ask for a second opinion on the mailing lists. Your code will need a security review before its inclusion.

• Obiously, code depending on either magic_quotes, register_globals, allow_url_fopen or allow_url_include PHP directives to be enabled, will never be accepted.