Wiki

Clone wiki

cloudengine / Coding_Guidelines

CloudEngine Coding Guidelines

Most of the coding guidelines below exist to make sure the code is easy to read and understand by other people. In some cases, the guidelines are purely conventions for reasons of consistency e.g. it is useful if everybody uses the same naming conventions - there is not necessarily a right or wrong way to name functions and variables, but it makes life easier if everybody does the same thing. There are also some guidelines designed to encourage 'good practices' in the codebase e.g. when it comes to security or accessibility. As a general rule we try to follow the CodeIgniter style guide except where otherwise noted.

There is inevitably going to always be some code in the codebase that doesn't meet all of these guidelines. Hopefully the amount of this is small and will be fixed over time, but don't necessarily use the code in the codebase as a guide to what the coding guidelines are, although you may still find it useful to look at the existing code to get a general feel for the conventions used. It's also worth noting that third party code such as open-source libraries is exempt from hese guidelines. All new core CloudEngine code should meet these guidelines nonetheless!

Our intent is that there shouldn't be 'unwritten' coding guidelines that you only discover when your code is being reviewed. In practice, a few things will probably slip through the net, and we encourage people who are reviewing code or whose code is being reviewed to suggest any additions and changes to these guidelines on the developer mailing list. Also, if anything in these guidelines is not clear or need elaboration, do also ask on the developer mailing list.

Many of the items below are 'guidelines' rather than 'rules'. There will be exceptions when it makes sense to break one of them, but you should have good reasons for doing so if you do!

Formatting

Files

  • Files should be saved with UTF-8 encoding. This is especially important for the MY_Language.php file and any PO files.
  • Files should use Unix line endings i.e. '\n'. You may find the EolExtension useful for converting line endings if you develop on a Windows computer.
  • The line length in files should generally not exceed 80 characters. It is fine to go slightly over this for strings inside t() functions and in views, but avoid excessively long lines nonetheless as they make the code much less legible.
  • Use spaces for whitespace not tabs (note: different from CodeIgniter style guide)
  • PHP files should omit the closing PHP tag since whitespace after a closing PHP tag can cause errors.
  • There should be no lines of whitespace at the end of a file.
  • Lines should have no trailing whitespace.
  • If you want to clean up whitespace in a file, then it is helpful if you do this separately from other code changes.

PHP Code

  • Indents should be four spaces (note: different from CodeIgniter style guide)
  • There should only be one PHP statement per line.
  • All binary operators (e.g. =, ==. <, &&) should have a space before and after the operator. There should also be a space before and after => when specifying an array.
  • There should be a space after commas e.g. in function declarations and when specifying arrays.
  • The formatting style should be as follows: (note: different from CodeIgniter style guide)
    if ($a == 1 && $b == 2) {
        echo 'a is 1 and b is 2';
    } else {
        echo 'a is not 1 or b is not 2';
    }

    if (!$a) {
        echo 'a is false';
    }

    foreach ($a as $key => $val) {
        echo $val;
    }

    switch ($a) {
        case 1:
            echo '1';
            break;
        default:
            echo 'not 1';
    }

    function do_something($a, $b = 1) {
        echo "$a $b";
        return $a;
    }
  • As well as the style for indenting braces, note
    • the whitespace after the if and while keywords, and after and before the else keyword.
    • the lack of whitespace after the ! operator,
    • the lack of whitespace on the righthand side of the leftbracket and lefthandside of the right bracket in statements such as if statements and foreach statements.
  • There should be one line of whitespace between PHP functions/methods.
  • A line of whitespace should be used to break up blocks of code in a function that belong together logically. e.g. leave a line of whitespace before and after a long switch statement.
  • When making assignments to variables, leave at least one whitespace on each side of the = operators and if making a number of assignments, align the = signs e.g
        $a   = 1;
        $ab  = 2;
        $abc = 3;
  • If an array needs to be line-wrapped, then align the '=>' symbols in the array items on separate lines.
  • If a line is more than 80 characters long, wrap the line with a carriage return at some point before the 80 characters in whatever way makes the code most legible.
  • Use TRUE and FALSE not true and false. Use $this->CI not $this->ci.
  • SQL commands should be capitalised. Try and break SQL commands that need to linewrap at logical points.

Code

PHP

  • All code should work in the PHP 5.3 and ideally also in older version of PHP 5. In particular any functions deprecated in PHP 5.3 should not be used. Support for PHP 4 is not required.
  • Methods in classes should be declared as public, protected or private. (Note much of the existing code needs to be fixed to meet this point).
  • The constructor in a class should be declared using 'public function __construct' rather than 'function MyClassName'. (Note much of the existing code needs to be fixed to meet this point).
  • If statements should always have curly braces, even if only one line long.
  • Use switch statements rather than long chains of ifelse statements.
  • Functions and methods should only have one return statement in them as a general rule unless the code would be significantly clearer with more than one return statement.
  • Be very careful about using the ternary operator a ? b : c. There are occasions when this is the most sensible way to express some code in a simple fashion, but use of ternary operators can easily lead to code that it is difficult to read. In particular, avoid nested ternary operators, using the ternary operator in place of complex if/else logic and using the ternary operator if the code involved would wrap over more than one line.
  • Make sure you have tested your code with PHP warnings turned on.
  • Make sure that you have handled any error conditions appropriately. Generally use the CodeIgniter show_error(), show_404() and log_message() functions as appropriate. Make sure that you catch exceptions from any third party libraries.
  • Strings should be single quoted unless there is a reason that they should be double-quoted (e.g. they contain a variable that needs to be parsed, or contain a single quote)

More generally

  • All code should respect the Model-View-Control structure of CodeIgniter. In particular, all database code should be in model functions - there shouldn't be any database functions outside the model files and e.g. you should not be having to call result() outside the model.
  • If appropriate CodeIgniter libraries exist then use them e.g. use the database library rather than trying to write database code from scratch!
  • Break code up into functions if it will help other people understand the code, even if this means that a function is only called in one place in the code.
  • All pages should have the title set in the controller, and the navigation section if appropriate
  • Form should be self-submitting as a general rule.
  • Forms should be validated appropriately using the CodeIgniter Form Validation library and happen in the controller.
  • GET requests should only be used for idempotent operations. POST requests should always be used for any operations that will affect data in the database.
  • Major new features should have a feature flag in system/application/config/cloudengine.php to allow the feature to be turned on and off via a flag in that config file. This is to allow site administrators to select which features they wish to use on their site and also to enable people to turn off features that have problems without having to shut down the whole site.
  • Configuration options should not be hardcoded but should instead be included in system/application/config/cloudengine.php
  • Make sure that any submitted content that should be moderated for spam (if the relevant feature flag is turned on) is done so.
  • Put URL segments in the arguments of controller methods rather than using uri->segment if possible, and always assign a default value
  • If your code requires database changes, then you should also add a protected function "_upgrade_NNNNN" to application/controllers/upgrade.php including at least one $this->message() call to inform admins what is happening during the upgrade. for the change.

Security

  • Make sure that any necessary permissions are checked e.g. that a user is logged in, has edit permission for an item, or admin permission as appropriate.
  • Where possible, CodeIgniter Active Record should be used. If that is not possible, because it does not support the query you wish to do, then you should be especially careful to make sure that SQL injection attacks are not possible and use the When using the query function, make sure that SQL injection attacks are not possible and use the CodeIgniter Database library functions for escaping queries.
  • Do not access GET, POST and COOKIE directly but instead use the CodeIgniter Input library.

Views

  • Use foreach:, endforeach; and if:, endif; rather than the more conventional PHP syntax in view files.
  • Except in the installer, use <?= in preference to <?php echo ...
  • Use the CodeIgniter Form Helper functions where appropriate, but do not use them if the HTML they replace is relatively simple or would be easier to understand to somebody already familiar with HTML. For example, it is probably clearer to use the HTML for a textarea than to use form_textarea(), but clearer to use form_dropdown() than to include the corresponding PHP and HTML.
  • All pages should validate in the W3C validator with no warnings.
  • Any javascript should degrade gracefully.
  • All styling information about fonts, colours etc. should be in CSS files
  • For hyperlinks/anchors, use the anchor function,
    • e.g. anchor("user/view/{ID}", "User profile")
  • For embeds/includes of dynamic content/controllers, use site_url function
    • e.g. <img src="<?=site_url("image/user/{ID}") ?>" ... />
  • For embeds/includes of static files, use the base_url function
    • e.g. <script src="<?= base_url().'_scripts/myscript.js' ?>" ></script>

Accessibility

  • Images should have appropriate alt tags. In particular decorative images should have an empty alt tag.
  • Users should be provided with an opportunity to provide accessible alternatives to any user-generated content that may not be accessible.
  • Use semantic markup e.g. use lists and heading where appropriate.
  • Make sure headings are properly nested for each page and that the list of headings on a page would allow the screen-reader user to navigate through the page.
  • Make sure your page titles are logical, descriptive and do not repeat information. They are used for bookmarks, by screen reader users and others.
  • Make sure that forms are built properly by using <label> tags with the id attribute. Text boxes should have the label coded before the input field. Checkboxes and radio buttons should have the label after the input field

String and internationalization

  • Always use the t() function around text strings
  • Always use the plural() function for plurals in strings.
  • Always use the t_link() function for links in strings.
  • Always use !required! in a string to indicated required form fields
  • Always use format_date for formating dates

Examples

<?=t("Your cloud !title has been created!", array('!title' => anchor("cloud/view/$cloud->cloud_id", $cloud->title))) ?>
<?=t("You can find answers... in [link-faq]our FAQ[/link].", array('[link-faq]' => t_link('about/faq_site'))) ?>
<?=format_date(_("Deadline: !date"), $cloud->call_deadline) ?>
<?=plural(_("!count view"), _("!count views"), $total_views) ?>

Internationalization Documentation

Comments

  • At the top of each file there should be a PHPDocs style comment identical to this, but with the file description, year and package changed appropriately. You can also include other PHPDocs tags, but please only use those officially documented. Author names should go in the CREDITS.txt file (where you are welcome to specify particular features that you have worked on) not in the files with
/**
 * Controller for cloud-related functionality
 *
 * @copyright 2009, 2010 The Open University. See CREDITS.txt
 * @license   http://gnu.org/licenses/gpl-2.0.html GNU GPL v2
 * @package Cloud
 */

(Note: the licence URL still needs to be added in existing files)

  • All functions and methods should have a block comment in PHPDocs style containing a function description, description for each argument and description for the return value.
  • Although code should be as self-documenting as possible, comments are clearly important, especially when they give a high-level view of the code or explain the reasons . As a general rule, comment why you have done something rather than describe what the code does.
  • Try to avoid abbreviations in comments e.g. use 'database' not 'db' and 'javascript' not 'js' and try to write your comments as sentences or at least shortened sentences as a general rule.
  • Don't put questions in comments - if you're not sure about something, instead put a comment like 'I have done X because of Y, but am not certain if this is the right approach because of Z'.
  • Don't use comments as a substitute for the issue tracker. It's useful to make a comment if you have put in a hack saying it's a hack and how it ought to be done. Any Todo comments must have an issue tracker number associated with them
  • Do not use comments as a means to remove old code. Please delete it rather than comment out code. It is confusing for other people coming along if there are commented out lines. It is always possible to get old versions of the code from the version control system.
  • If you don't want to totally delete debug you have put in, don't just comment it out, put in a debug flag. As a general rule, only keep debug in the code for particularly tricky bits of code that are likely to cause problems
  • Comments should ideally use British English unless being consistent with US English used elsewhere.

Naming conventions

  • All names should ideally use British English unless being consistent with US English used elsewhere.
  • Be very careful about any abbreviations in naming - don't use them unless they are extremely obvious and a significant shortening.

Files

  • The names of model, view and controller files should be lowercase, with words separated by underscored if necessary.
  • Controller names should be singular e.g. cloud.php not clouds.php.
  • The names of model files should be of the form cloud_model.php.
  • View files should be separated into appropriate directories and e.g. called cloud/edit.php not cloud/edit_cloud.php and cloud/view.php not cloud/view_cloud.php.
  • View files for form submission success should be named e.g. edit_success.php and view files for confirmation of form actions should be name e.g. delete_confirm.php. More generally, try and be consistent with the naming for existing views.

Functions and methods

  • Function and method names should be lower case with words separated by underscored.
  • Private functions should start with an underscore
  • In controllers, except in the API controller, use view, add, edit, and delete as the function names for viewing, adding editting and deleting items.
  • In models, use get_item, insert_item, update_item and delete_item as the function names for getting, inserting, updating and deleting items (with item replaced by the name of the item in question).

Variables

  • Variable names should be lowercase.
  • Use meaningful variable names. Variable names such as $i should only be used for e.g. loop counters.

Database

  • Table names should be lowercase with underscores separating words if necessary, and should be singular if appropriate.
  • For tables that cross reference two other tables use the name of the two other tables separated by an underscore e.g. cloudscape_cloud.
  • Column names representing IDs should always be prefixed e.g. cloud_id not id to avoid confusion
  • Use created, modified as the column names for when items were created and last modified. Use timestamp only as the column name for events.

Database conventions

  • All ID columns should be int 10, unsigned, not null.
  • Timestamps should be recorded as unix timestamps as an int 10. (Note: this is not currently the case in the user* tables for historical reasons, and is unlikely to be fixed before a major new release)

Graphic Design

  • The user interface for any new functionality should match the style of the existing user interface.
  • Generally try to use the existing CSS styles rather than adding new ones unless there is a definite requirement to do so. If you do need to edit the main stylesheet, then also change its version number in the name of the style sheet. This is to prevent problems with caching of old stylesheets.

Third party code

  • Any third-party open-source code added must have a licence compatible with GPLv2 and details must be added to the CREDITS.txt file.
  • Be very careful when using snippets of code from websites to check the licensing terms for them.

Updated