Lesson #2 Class Notes wiki -- Coding Standards, Keeping code clean

We encourage users to post events happening in the community to the community events group on https://www.drupal.org.
You are viewing a wiki page. You are welcome to join the group and then edit it. Be bold!

Plan

  • Sound Check: make sure everyone can get on Skype and is in IRC
  • Debrief: talk about how we've been doing, also about moving the regular lesson time to Sunday and/or starting other times
  • Drupal Coding Standards: what they are; why they matter
  • Keeping Your Code Clean: where to put stuff; how not to kludge
  • Keeping Your Code Safe: how and why to use Drupal functions for text, links, input fields and database queries
  • Making Your Code Intelligible: the tao of commenting
  • Dojo Challenge!: review a patch for Drupal 5.0
  • Documentation: lets keep the great documentation coming!

For documentation references used throughout the lesson, see glendac's great Lesson 2 Pre-Lesson Resource List

Sound Check: make sure everyone can get on Skype and is in IRC

This worked pretty well, but some people reported problems. Support throughout lesson.

Debrief: talk about how we've been doing, also about moving the regular lesson time to Sunday and/or starting other times

Next lesson on Sunday at convenient time (taking into all the different timezones, international solidarity and Josh's new lifestyle), see Dojo Lesson #3: First Sunday Training for details.
Others may teach on other days, the interesting thing will be, who will be the first non-Josh person to step up and teach a class? eCommerce or project module?
The Group has more than four hundred people subscribed, we have artwork, things are looking very good.
Drupal use is doubling each year. Group much bigger than expected due to intimidating character of what is needed to learn. So there is a lot of excitement generated around the group.
Thanks to everyone making the audio and everything possible.

Drupal Coding Standards: what they are; why they matter

Drupal coding standards page
Drupal coding standards based on PEAR standards. It's about style, how do we handle indenting?
Don't use tabs (or configure your text editor to have tabs make spaces automatically if you are addicted to the tab key). Two spaces look the same everywhere whereas tabs can look wildly different according to text editor settings.
There's a whole section on how to manage control structures.
All of this is important because the code that you write hopefully is going to contribute back to a larger project that a lot of other people are going to work on.
Everyone has a tendency to want to code their way, as they have been doing for years and years, without commenting, but there are good reasons for coding standards, especially in large group projects of this kind.
So using coding standards makes code (modules and themes) much easier to read no matter who wrote them, it makes it easier for collaboration among a group of developers, it makes it easier to read, it makes for ease of use and helps development, there are fewer errors. It really helps you write clear, secure, well-functioning code. Writing code consistently the same way every time is a great way to make sure you are doing things right and that other people can see what you are doing.
It's about setting standards, and that is one of the things that Drupal does very well.
It has made me a better coder, to use these standards and to use this style.
The main thing is how to declare functions, handle control structures, it's all covered in Drupal coding standards page: http://drupal.org/node/318

Style

  • Indent with two spaces
  • Use a space between the equals sign and the thing you are assigning, in array assignments, after the comma, between the key and the value in an associative array, indent the members of a form array (it's impossible to read if you put it all on one line).
  • Comment your code (makes it easier to understand even one's own code six months later).
  • Single-line comments:
    // Comment
  • Multi-line comments:
    /**
     * Comments
     *
  • Doxygen commenting using the @ symbol for automatic inclusion of comments in API or API style generated documentation (this example from ./includes/common.inc:

    <?php
       
    /**
         * Set content for a specified region.
         *
         * @param $region
         *   Page region the content is assigned to.
         *
         * @param $data
         *   Content to be set.
         */
       
    function drupal_set_content($region = NULL, $data = NULL) {
          static
    $content = array();
       
          if (!
    is_null($region) && !is_null($data)) {
           
    $content[$region][] = $data;
          }
          return
    $content;
        }
    ?>
  • Including code: always use include_once(). Including code helps you keep organized and prevents unneeded code from getting loaded. Drupal sometimes runs through functions multiple times, so it is important to use include_once(), so as not to load code multiple times.

  • Use <?php ?> and not the shorthand <? ?>
  • However, we omit closing code files with ?> in order to prevent whitespace from breaking things.
  • When using CVS, include the Id CVS Keyword

Naming conventions

  • Functions and methods should be named in lower case, with underscores used for word separation
  • Avoid CamelCase and Hungarian notation like the plague
  • We are technically "wasting" space on the underscore, but readability makes a big difference.
  • Functions should have the grouping/module prefix separated by an underscore to avoid name collisions between modules. Read about similar conventions for variables, constants, etc.
  • Documentation files should have the *.txt filename extension.

So all of this constitutes the Drupal Community Standards on how to write code, for committing patches back to code and core people are strict about these standards and they are strict about it for a reason, and this is one of the reasons Drupal is as valuable a project as it is and things can be shared so easily.

Keeping Your Code Clean: where to put stuff; how not to kludge things

Last time we spoke about putting code that outputs HTML into a theme function that can be overriden, so that people who put up sites using your module can control how the output looks, which is so critical. And you don't want to have to hack the module to do that, it should be possible to just write a theme function.
In every theme directory, Garland for example, there are a few standard files that are in almost all themes. Last week we were looking at the node.tpl.php file, and at a node{-cck content type name}.tpl.php file. There are some other standard files too: style.css is of course where your css goes for the theme.
Another really important file in the theme system is template.php. If you are putting up a site and you don't really have a module, but you need to write a couple of functions that you use over and over again, template.php is a good place to put them.
Anything you put in template.php will be available system wide, and this is where you put theme overrides.
The file page.tpl.php is the master template for your theme, and in many sites you put a lot of modifications there, for example, on certain pages I want to show the title, and on others I don't... You can "code the heck" out of page.tpl.php, which will make the site hard to maintain: conditionals here, thirty lines of code there, it can get to be a mess, and hard to maintain.
One way to avoid that problem is to have functions reside in template.php, and then just point to them from page.tpl.php.
Let's suppose we want to set up an alternative in the primary links on the front page.
Before:

    <?php if (isset($primary_links)) : ?>
        <?php print theme('links', $primary_links, array('class' => 'links primary-links')) ?>
    <?php endif; ?>

After:
    <?php if (isset($primary_links) && drupal_is_front_page()) : ?>
      <?php print theme('links', $primary_links, array('class' => 'links primary-links')) ?>
    <?php }
      elseif(
strstr($_GET['q'], 'node/2')) {
        print
'<b>¡Hola, mundo!</b>';
    <?
php endif; ?>

The API function drupal_is_front_page() is handy.
$_GET['q'] tells us what the system level path is plus any arguments that are being sent

Diversionary note: Pretty much every where in core code, to see what's in the URL, the arg() function is used. You'll almost never want to go directly to $_GET['q']. So, to see if we're on 'node/2', use something like elseif (arg(0) == 'node' && arg(1) == '2'). To see this in action, look at node_menu().

Well, these types of control structures are useful but they can be addictive. If you have eighteen or twenty of these types of statements, and instead of Hello World you are actually printing out a table, or something useful, or a series of divs, it can get to be a little hard to manage.

Better:

  • page.tpl.php:
        <?php if (isset($primary_links) && drupal_is_front_page()) : ?>
          <?php print theme('links', $primary_links, array('class' => 'links primary-links')) ?>
        <?php }
          elseif(
    strstr($_GET['q'], 'node/2')) {
           
    theme('hello_world', $title);
        <?
    php endif; ?>
  • template.php:
    <?php
        phptemplate_hello_world
    ($title) {
          print
    '<b>¡Hola, mundo! I\'m a special page, called '. $title .'</b>';
        }
    ?>

    Note the use of single quotes and spaces according to Drupal programming style. Easier on the eyes and easier on the old processor.

So, the importance of template.php in a site's theme. It is a good place to put all around stuff available system wide and as theme override functions.
Because otherwise, a lot of coding is going to be bound up with the HTML of the specific node being dealt with on the presentation level, and apart from being hard to read and messy, separating the code out into something like template.php gives you the option of easily abstracting portions into a future module, etc. And for groups of graphic designers working elbow to elbow with coders, the separation of tasks is much easier and less error prone.

So, template.php is a perfectly acceptable place to put theme function overrides, invocable on higher level theme files.

As a second example, we will override a second theme function in template.php, this time theme_username(). As a method, to override it, we go to http://api.drupal.org/api/HEAD/function/theme_username and copy the existing code, paste it into template.php, prefix the function with phptemplate_ instead of theme_ and go from there:

<?php
function garland_username($object) {

  if (
$object->uid && $object->name) {
   
// Shorten the name when it is too long or it will break many tables.
   
if (drupal_strlen($object->name) > 20) {
     
$name = drupal_substr($object->name, 0, 15) .'...';
    }
    else {
     
$name = $object->name;
    }

    if (
user_access('access user profiles')) {
     
$output = l($name, 'user/'. $object->uid, array('title' => t('View user profile.')));
    }
    else {
     
$output = check_plain($name);
    }
  }
  else if (
$object->name) {
   
// Sometimes modules display content composed by people who are
    // not registered members of the site (e.g. mailing list or news
    // aggregator modules). This clause enables modules to display
    // the true author of the content.
   
if ($object->homepage) {
     
$output = l($object->name, $object->homepage);
    }
    else {
     
$output = check_plain($object->name);
    }

   
$output .= ' ('. t('not verified') .')';
  }
  else {
   
$output = variable_get('anonymous', t('Anonymous'));
  }

  return
$output;
}
?>

Now, we could prefix with a specific theme name ("inheritance" lower down and more specific on the theme engine hierarchy) rather than phptemplate_username(); for example, garland_username().

Suppose we want username in plain text, plus contact this user as a link (we make sure the contact module is enabled first). So, instead of:

    $output = l($name, 'user/'. $object->uid, array('title' => t('View user profile.')));

we put:
    $output = check_plain($name) .' ('. l('contact', 'user/'. $object->uid .'/contact', array('title' => t('Contact this user.'))) .')';

So instead of a link to the user profile page on the posting information, you get the user name in plain text along with a link, in parenthesis, straight to the user's contact page.

Other things you can do here is to have user profile fields with Skype names, and then you can add a routine to check and see if they are online and if so you can put an appropriate icon next to the name and have it as a link directly to instant messenger.

So this is stuff you do in template.php to override existing theme functions. Check out snippets in the Drupal Handbook to do interesting things overriding theme functions.

Question: Where to put third-party modules (that means us!)

Use of ./sites/all directory to place contributed modules
default – mysite.com

Question about Multisites

The whole thing about multisites is that if you want to run several sites off of the same codebase you should be able to do that.
From the comments in ./default/settings.php:

 * For example, for a fictitious site installed at
 * http://www.drupal.org/mysite/test/, the 'settings.php'
 * is searched in the following directories:
 *
 *  1. sites/www.drupal.org.mysite.test
 *  2. sites/drupal.org.mysite.test
 *  3. sites/org.mysite.test
 *
 *  4. sites/www.drupal.org.mysite
 *  5. sites/drupal.org.mysite
 *  6. sites/org.mysite
 *
 *  7. sites/www.drupal.org
 *  8. sites/drupal.org
 *  9. sites/org
 *
 * 10. sites/default

And we should now add:

 *       sites/all

Josh says that in some sites, he has gotten rid of sites/default entirely, to avoid unforeseen wildcard invocations of default settings, modules and themes.

The .{drupal installation root}/files directory is potentially accessible to any of the multisites, but they would have to know the url of another site to access its files.
However, you can use the Drupal Administration interface to tell each individual site installation where drupal files exist, in the administration settings (administer > Site configuration > File system) specify "File system path" for each site. Instead of that being "files" it could be, for example, "site/mysite.com/files", where you can actually create the files directory (with the appropriate operating system permissions to make sure Drupal (the http server) has read/write access).
For each site, the settings.php configuration directs Drupal to different databases, giving us the possibility of having the "file system path" setting different for each site, if that is what you want to do.

Question: What is the ./all directory for, as opposed to ./default?

Answer:
Starting with Drupal 5.0, in the ./sites directory you can have ./all and ./default, and if you have contributed modules you want to make available across several sites, you can put them into the ./all directory (read the README.txt in the ./all directory!).
./all is for modules, themes, etc. to be made available to all sites, no settings.php
./sites sets up possibility of site specific modules and theme availability along with site specific settings.php
Different stacks of modules can be made available to different sites
You can have one module pointing to one database cluster, for example, while others... (load balancing).

On the importance of includes

If a module is very big, for example the views module, use include “.inc” files, which help keep code nicely organized and also are to be encouraged when collaborating with others in development.
Check out http://cvs.drupal.org/viewcvs/drupal/contributions/modules/views/ to see views code, for example, and its use of *.inc include files.
// start webchick comment
[If] your module is enabled, that .module file will be loaded into memory on each page request.
Therefore, if you have tons of code that _doesn't_ need to be executed on each page request
Like, for example, a series of functions that connect to some external service that are only relevant on pages that you are actively using your module on...
Then it can be best to put them into .inc files or so.
.incs can also be useful if you want to provide an API for other module developers who might not necessarily use your module, but might want to use library functions it provides.
//end webchick comment
sepeck: splitting the UI portion helps segragate permissions as a side effect and reduce the loading of code when you've done all the configuration stuff and don't need it anymore too
sepeck: I set up my views, then turn off views_ui

Keeping Your Code Safe: how and why to use Drupal functions for text, links, input fields and database queries

What makes code unsafe?
Cross site and other scripting attacks due to improperly checking output.
Also using php to monkey with the lower level aspects of the system.
Allowing people to gain access to nodes and the system.
SQL injection attacks where someone puts in some escape characters into a text field and manages to execute a SQL query of their own devising, very often to delete what they can as a form of vandalism.
Cross site scripting attacks to sniff your session and obtain root access privileges to the Drupal system and even hack at the server itself.

preventing people from vandalizing your site

http://drupal.org/node/62304 Writing secure code

Handling text in secure fashion.

check_plain, check_markup etc. ( see: http://api.drupal.org/apis/HEAD/check ) are ways of specially checking the input apart from the node_save() function, which will also run all the enabled filters.
Another place to use Drupal functions is whenever you are doing a database query.
Rather than putting some external variable into a raw SQL query, use this: db_rewrite_sql(); it is more secure, runs the query through a simple filter designed to prevent SQL injection and invokes node permissions.
See When to use db_rewrite_sql()

Parametrized query prevents SQL injection

put this in template.php:

    function dojo_query() {
      $sql = 'SELECT * from {node} WHERE nid = %d';
      $sql = db_rewrite_sql($sql);
      $result = db_query($sql,2);
      $node = db_fetch_object($result);
      print_r($node);
    }

Run that from php execute (devel module).

One gets used to this way of programming and it becomes quite productive.

Of course, if one is doing some coding that doesn't involve interaction with users or the screen, collecting internal stats, for example, then it is not necessary to use the db_rewrite_sql() function.

For more information, consult the Drupal Handbook and the API documentation.

Other Drupal helper functions that also make your site safer

t() function

Localization engine; t() translates strings to current locale, localizing the string.
Any time you output semantic text (localization) using this function, someone will be able to translate your module to, say, French.
Localization is good for localizing English to local application specific lingo: rename book module to something more apropos to the user base.
Make your own mysitetalk language. So "books" can be called "index cards", for example.
You have to enable the locale module for this functionality.
It is conceivable to use localization to localize even images.
Examples in the t() API docs.

l() function

Used for making links.
Instead of <a href=”something”>... use l(), so that "something" is contextualized not just from the front page, but instead as a node.
In addition, you are liberated from having to worry about whether Drupal is installed in the root document directory of a site or in a subdomain or in a subdirectory, or whatever path it may have in absolute http terms.
Of course, in programming themes, one has a series of variables and functions for constructing the installation-independent path; but in module programing, one must use the l(function).
See Drupal API documentation.

Question: difference between url() and l() functions

url() generates url from a drupal menu path (not a link).
So, the l function uses the url function, then makes it a link.

Question: difference between localization and internationalization

Internationalization (i18n) is different from localization.
Localization, part of drupal core, allows you to translate system text.
Text generated from within modules will be localized according to settings.
i18n allows you to have multiple translations of a node
See internationalization group at http://groups.drupal.org

Use Form API for safe code!!!

The Drupal community is obsessed with security to prevent people from hijacking Drupal sites.
For example, form_token (works to prevent, say, simple spam bots from dropping stuff on you)
Just take a look at the HTML source of any ./node/add/page (?q=node/add/page if you are not using clean URLs) to see the kind of safeguards that are taken.
You are letting people type in stuff that is being sent to the server...
samtresler: QOTD:

If you don't use formAPI you are entering a World of Pain!"

//samtresler: While I'd love to take credit for this, I was just typing what josh_k had said on Skype.... although I happen to agree with him.
//victorkane: Right, Sam, as Josh says (I could barely make it out from listening to the podcast), it's actually a quote from the movie "The Big Lebowski": See Memorable Quotes from The Big Lebowski

Making Your Code Intelligible: the tao of commenting

It's better to comment your code. Check out Doxygen style comments.
Comment on the purpose of functions.
Say which hook you are implementing.
Browse the code of well-known modules (like CCK) to check out their use of commenting.

Dojo Challenge!: review a patch for Drupal 5.0

How to put this to work.
Best practices.
See webchicks post: Some easy first core patches for some lucky someones!
Drupal needs right now: review patches.
check the work.
Social convention: people who maintain don't just stick in patches, people review it first.
There are 60 or so patches in the queue that need committing.
For next week, help to review patches.
1. Get test envoironment set up: http://drupal.org/node/28245
2. update.php for each module change
3. review a patch
4. code style clean up (spaces, ...)
5. to apply patch: patch -p0 < profile-cleanup.patch
6. ajk^: yes, when creating core patches, make them from the drupal/ directory :)
7. when reviewing them, include the why you are +1 or -1. Just saying +1 this is neat... doesn't actually help
8. how to roll a patch... http://drupal.org/patch Instructions for all of this
Take a look at the patch queue, and try reading one.
There are 60...
Release Drupal 5.0 on the 14th in time for Drupal's birthday.

Documentation: lets keep the great documentation coming!

Thanks to all who contributed

advertisement:

If you have existing consultancy, or are interested in doing so, want to work, moonlighting... (Josh, please edit this part :)
drupaljobs at chapterthreellc dot com

PHP books recommended on IRC during lesson

Advanced PHP Programming by George Schlossnagle is a pretty good book..
WimLeers: I've taught myself basic php using Paul Hudson's phpbook (also available on the internet: http://hudzilla.org/phpbook)similarities between his advice and what i'm learning here