Common developer mistakes in Drupal

A small list of the kind of things experienced Drupal developers hate to encounter on a site and why they suck.

This list is far from comprehensive.

Screwing around with core schema

Update 3/04/10: This got left out of the original list because I always assumed this was so obviously bogus I'd never actually run into this in the wild. I've recently been proven wrong by a community contributed module available for download from Drupal.org(Force Password Change RC4 for those curious).

So, for the cheap seats: DO NOT MODIFY TABLES CREATED BY CORE MODULES.. It might seem like a nifty way to get something done but all you're really buying for your trouble is a broken, unsupportable database that can't be migrated. For extra credit expect contributed modules (or even chunks of core) to break in mysterious ways.

My Kung Fu is Best!

Over time every developer comes up with their own particular code style and while this is a natural part of learning the craft no other topic (with the possible exception of which text editor is best) has lead to more arguing and wasted productivity.

Tabs vs spaces, brackets on the same line or below control statements or correct use of comments and whitespace, whatever camp you happen to be in one thing holds true for all parties: nothing is more irritating than having to read through someone else's code, especially if their style is substantially different than yours.

Fun fact: with a few thousand developers from around the world the Drupal project can't afford to continuously revisit these kind of bullshit arguments, thus the Powers That Be have graced the community with a set of coding standards.

Even better, the Coder module has been provided to make adhering to the community standards even easier.

So, for the people in the back row: the community at large doesn't give a flip about your personal preferences about code formatting. We don't care if you think the style rules are the moral equivalent of using flaming kittens to light cigars rolled from the leaves of the Bodhi tree. You're free to format your code any way you want on your own time but if you plan on contributing to Drupal write to the community standards.

Module Bloat

This really isn't developer-specific but in my experience this mistake is so prevalent that I'm compelled to mentioned it anyway.

Installing a ton of modules is an excellent way to murder your site's performance. Sure, it's easy and fun to extend your site's feature set by installing just one more module, and with ~5000 to choose from there's plenty available. One thing that needs to be kept in mind though is every module installed represents a tradeoff between functionality and performance.

PHP FTW

Gratuitous use of stock php functions to get stuff done when there are core API functions available that do the same thing. I've seen this a lot in situations where you have a skilled developer who's unfamiliar with Drupal and operating under a deadline.

This a problem because now you are saddled with a bunch of unnecessary custom code to maintain that isn't Drupal aware. This means it can't be extended or worked with via the normal set of Drupal hooks or contributed modules and everything has to be done the hard way.

The Drupal core API is designed to make life as a developer easier. You stand to benefit from learning and using it.

node_load() Considered Harmful

Apparently little known fact: node_load() invokes every node data hook (and associated database calls) in your codebase. Needless to say this is NOT cheap. This is awesome if you need to display just about every piece of information in the database for a single node. This sucks royally if all you where after is the node title. But don't take my word for it, see for yourself:

A practical demonstration:

• In your development environment, install the devel module and turn on the display for query logging & page execution time.
• If you haven't already done so enable the php filter module
• Create a node with the body left blank and save it. Note the number of queries generated on the node page.
• Now edit the node and put the following php in the body
  

  <?php
  $nids = db_query('select nid from node limit 10');
  while($nid = db_result($nids)) {
    $my_node = node_load($nid);
    print $my_node->title . "\n";
  }
  ?>
  

• select the php filter and save the node
• Note what just happened to your query count and page execution time. Yeah. This gets even worse when you start installing modules that add data to nodes. See Module bloat.

Alternative to node_load()
If you don't need the entire node object most of the time it's a lot cheaper to just query the database directly for just the information you need, then parse it out of the results returned.

Note: the same holds true for user_load() and other large data object loaders in Drupal.

Easter egging code

I once lost most of a day figuring out how a list of blog posts where being displayed on the homepage of a site I was working on. The previous developer had created a custom block that referenced a function declared in one of the include files for the theme. The function in question was little more than a wrapper for views_load_view().

I've lost count of how many times I've had to come in behind another developer to debug weird database and/or user authentication issues only to find a nest of code in settings.php that was breaking stuff.

I've since learned that doing a grep -R 'function <function name>' * within /sites is a quick way to track stuff like this down but that really isn't the point,

Fun places your code doesn't belong:

• settings.php - it's a configuration file. Under what conditions does it make sense to stash code in a configuration file? If you need to get some code running high in the execution stack try hook_init().
• theme files - Here's a quick test: if you can change the theme to Garland and break functionality someone is doing it wrong.
• nodes - seriously, what part of hiding code in the database sounds like a good idea?
• Standalone PHP files - having to grovel around for non-module code you've pulled into the site using include() or whatever is ridiculous. Having to read someone's halfassed CakePHP implementation of an existing feature is even more so. Write a module.

The big takeaway here should be this: Drupal is (in general) pretty well organized. There's generally a known good or otherwise generally accepted place for your code. 99% of the time that's going to be a custom module. Stashing code in strange places doesn't buy you anything, it just makes your site a bastard to support.

Around the world, around the world...

Too. Many. Globals. And they are usually poorly namespaced as well. Here's an example: I recently ported a module written by a third party that declared 16 global variables, all of which related to the same task. Out of the 16 none of them where correctly namespaced. I ended up collapsing all of these into a single global keyed array.

This is the kind of thing that variable_set() and variable_get() are designed to eliminate. If you really must use globals, keep it to a bare minimum and make sure to adhere to the community best practices for defining globals.

FrankenDrupal

Usually the product of unscrupulous contractors or developers that are in way WAY over their head. this refers to any poorly concieved attempt to integrate a 3rd party service or standalone php code (non module .php files) that replicates core or common contrib module functionality without the benefit of a module or set of modules to provide a bridge between the two.

This usually involves core code getting hacked or replaced wholesale and is a guaranteed way to make a site 100% unsupportable. No one in the Drupal community is going to touch this kind of botched integration; excepting perhaps with pitchforks and burning torches as Frankendrupal contemplates its unnatural existence high in a tower on the hill.

Key takeaway: if Drupal isn't the tool you need, use something else. If Drupal IS what you need and you don't know how to do something, ask for help.