Drupal Maintainability II - Separation of Church and State

This week's blog post on Drupal maintainability is about keeping things separated (You can take a look at the other posts in the series). Drupal is really powerful. One of the core assumptions is that every site will be different and so everything can be manipulated and extended to your heart's content. And for any given change that you'd like to make there are many ways that you could go about it. However, some paths are more holy than others. How do you pick the road that will lead to Drupal salvation?

If finding the enlightened way is something you desire to achieve, then design patterns are something you need to study. Now I'm not talking about knitting, or crochet patterns, I'm talking about code design patterns. There are dozens of design patterns, some more useful than others, but there are a few that you must need to know. One of those is Model-View-Controller (MVC).

Now for those of you who are about to click away in fear of a lofty treatise on software theory fear not; this post is going to be about pragmatic application, specifically in Drupal. I don't even have a computer science degree.

MVC is an architectural design pattern. It's about how the application as a whole is structured and not so much about the atomic parts. You'll see the term bandied about all over the Internets, and like most acronyms that get bandied about, it's lost a lot of its meaning. But it's all about keeping the various aspects of the code separate.

Lets start with looking at code that is not MVC and then work our way towards the light. Here's a page callback:

<?php
function my_module_widgets_page($id) {
 
$res = db_query('SELECT foo, bar, count FROM {widget} WHERE widget_id = %d', $id);
 
$header = array('foo', 'bar');
 
$rows = array();
 
  while (
$widget = db_fetch_object($res)) {
   
$rows[] = array($widget->foo, $widget->bar, $widget->count);
  }
 
 
$output = "<p>Here's some widgets.  If you don't like these there's "
   
l('zorogs', 'zorogs') .' that you can check out instead.</p>';
 
$output .= theme('table', $header, $rows);
 
$output .= '<p>I hope you like our widgets</a>';
  return
$output;
}
?>

Now while this chunk of code creates semantic markup, (apologies on behalf of GeSHi for mangling the code spacing). First off what if I don't like the markup? What if it doesn't conform to my site's style guidelines? What if we also want to have the data in XML or jSON? The HTML here is almost etched in stone just like with Moses's stone tablets. Moses smashed his first set of tablets, we should do the same.

Secondly what about that SQL statement? What if we want to load widgets somewhere else? While this particular SQL statement is pretty straightforward to duplicate, the key word there is duplicate. If you duplicate something then it becomes more difficult to manage all these duplicates. Especially if you have many minor variations.

So back to MVC. We better get some definitions before we get too much further.

Model
is the part that handles the manipulation of raw data. If you've heard of CRUD (no not the stuff under your fingernails) this is the kind of stuff we're talking about. In Drupal core this is things like node_load() and user_add().
View
is what renders something. Think tipple piffs (.tpl.php files), and theme_ functions.
Controller
This bridges the gap between the two. This handles the interaction between the user and the Model. In core think menu callbacks or FAPI.

And never shall they become intermixed. Except just like church and state, it's not always that easy to draw that line — I'm sure there some of you who are questioning my examples above.

So how do we decide what goes where? There's some general rules, but like all rules, they should be broken from time to time. Looking at our example above, anywhere that you have HTML tags, or anywhere that you are using a variable like $output is probably a good candidate for moving into a theme function (the View). Anywhere that you have some SQL is probably good to through into an APIish function.

Now before you themers become bored and head instead down the dark paths of debauchery (though it's probably too late for that), know that there's hope here for you too. You might be tempted to just assume that the theme is the View and dismiss the rest of this as developer stuff, but you'd be wrong. While we definitely don't want to see any Model stuff in the theme, there can be plenty of Controller.

Let's say that you've got an multi-value image field and you want to display the first image at the top of the body, and the remainder at the bottom. While You might just throw a bit of PHP into your node-widget.tpl.php this has drawbacks. Let's say you also need to do this for the zorogs node type, you now need to duplicate that chunk of PHP in multiple places. Read maintenance problem. What if you've got to do other similar manipulations? That tipple piff is going to get ugly real quick. So use pre-process functions to keep things manageable and to reduce duplicity. It's really a whole blog post on it's own. But if you haven't already, take a look at the Zen theme. It make pre-process (and a host of other things) easy.

Back to the developers and their recalcitrant hearts. Now just throwing your SQL into a separate function does not an API make. Well it does, it'd just be pretty terrible. Here's a terrible API function:

<?php
function my_module_widget_create($widget) {
 
drupal_write_record('widget', $widget);
 
drupal_set_message('Another widget created.');
}
?>

For starters there's no doxygen comments so how's anyone supposed to know how to use the thing? You're gonna come along in two days wanting to make another widget and you won't remember what exact parameters you need to pass. Secondly there's no validation checking. Is there going to be fire and brimstone if a widget has no foo? If so then check for it. Finally what's with that drupal_set_message(). This sin is all too common in supposed "API" functions. What if I'm creating widgets in the background and I don't want the user to know? Having messaging in an API function makes it useless anywhere other than the place it's original called. Here's what we could do instead:

<?php
/**
* Create a widget.
*
* @param object $widget
*   a widget object, must have a property for foo.
* @return boolean
*  TRUE on success, else FALSE.
*/
function my_module_widget_create($widget) {
  if (empty(
$widget->foo)) {
   
watchdog('my module', 'A caller tries to create widgets without foo.  Examine thy heart oh caller.');
    return
FALSE;
  }
  return (
drupal_write_record('widget', $widget) == SAVED_NEW);
}
?>

This way the caller can decide how to respond to a success or failure.

That's all for this week. Leave your tithes in the box on your way out, and come back next week for more words to bring you down the path of maintainability.

Besides Drupal development, Advomatic offers a wide range of other Drupal services, including maintenance and clustered hosting. Contact us for your next project.

IceCreamYou wrote 5 years 14 weeks ago

This is a well-intended post that makes some good points... but you should use drupal_write_record() instead of db_query("INSERT..."). ;-)

Dave Hansen-Lange wrote 5 years 14 weeks ago

Good point. That's a new Drupal 6 function that I often forget about. I'll edit accordingly.

Larry Garfield wrote 5 years 12 weeks ago

Separation of concerns is good. That does not imply MVC. Drupal is not MVC. See: http://www.garfieldtech.com/blog/mvc-vs-pac

Contact Us

About Dave Hansen-Lange

Dave Hansen-Lange has been developing websites since 2003 when he needed a web presence for the record label that he founded with several fellow musicians.

AdvoTwitter