Man, it's quiet around here. Interested in doing some pimpin'?

Man, it's quiet around here. Interested in doing some pimpin'?


Code pimping. You know? The thing I'd discussed before? Forgive my earlier informality. I see now how my words could have been confusing.

The very first edition of Pimp My Code is special because the code we’ll be looking at will be included in Prototype 1.6.1. (It's a bit like if we were to Pimp [someone's] Ride™, then decide to keep the car for ourselves.) So this is more than just an academic exercise for us — the “pimped” result is now part of the Prototype source code.

The original

The code in question, from Sébastien Grosjean (a.k.a. ZenCocoon), implements element “storage” — attaching of arbitrary data to DOM nodes in a safe and leak-free manner. Other frameworks have had this for a while; jQuery’s $, for instance, is used heavily by jQuery plugin authors to great effect. But Seb’s is based on the similar Mootools API, which I’ve admired since it debuted in Mootools 1.2.

Here’s Seb’s code. It’s a long code block, since he’s been thoughtful enough to comment the hell out of it:

The idea is this: instead of storing arbitrary objects as properties on DOM nodes, create one custom property on the DOM node: an index to a global hashtable. The value of that key in the table will itself be a collection of custom key/value pairs. On top of avoiding nasty IE memory leaks (circular references between DOM objects and JS objects), this has the benefit of encapsulating all of an element’s custom metadata into one place.

Let’s make a first pass at this, line-by-line.

The critique

Object.extend(Prototype, {UID: 1});

Already we’ve gotten to something I’d change. Seb is using the Prototype namespace correctly here, in that he’s storing something that’s of concern only to the framework and should feel “private.” But my own preference is to move this property into the Element.Storage namespace. I am fickle and my mind is hard to read.

Element.Storage = {
  get: function(uid) {
    return (this[uid] || (this[uid] = {}));
  init: function(item) {
    return (item.uid || (item.uid = Prototype.UID++));

OK, another change jumps out at me. The Element.Storage.init method gets called in both Element#store and Element#retrieve; it handles the case where an element doesn’t have any existing metadata. It creates our custom property on the node and increments the counter.

In other words, store and retrieve are the only two places where this method is needed, so I balk at making it public. My first instinct was to make it a private method inside a closure:

(function() {
  function _init(item) {
    return (item.uid || (item.uid = Prototype.UID++));
  // ... rest of storage code

I started down this path but quickly stopped. Instead, we’re going to refactor this part so that the init case is handled without the need for a separate method. Let’s move on for now.

Element.Methods.retrieve = function(element, property, dflt) {
  if (!(element = $(element))) return;
  if (element.uid == undefined) Element.Storage.init(element);
  var storage = Element.Storage.get(element.uid);
  var prop = storage[property];
  if (dflt != undefined && prop == undefined)
    prop = storage[property] = dflt;
  return prop;

A few things to mention here.

  • Variable naming is important. The ideal name for the third parameter of this function would be default, but that’s off-limits; default is a reserved word in JavaScript. Seb’s opted for dflt here, which is clear enough. I’d change it to defaultValue because I like vowels.

    As an aside: my first instinct was to remove the defaultValue thing altogether, because I was surprised by the way it behaved. I didn’t find it very intuitive to give Element#retrieve the capability to store properties as well. So I took it out.

    I changed my mind several minutes later, when I wrote some code that leveraged element metadata. I had assumed I wouldn’t need the “store a default value” feature often enough to warrant the surprising behavior, but I was spectacularly wrong. I put it back in. Consider that a lesson on how your API design needs to be grounded in use cases.

  • The idiom in the first line is used throughout Prototype and (and, in fact, should be used more consistently). It runs the argument through $, but also checks the return value to ensure we got back a DOM node and not null (as would happen if you passed a non-existent ID). An empty return is equivalent to return undefined, which (IMO) is an acceptable failure case. Bonus points, Seb!

  • The custom property Seb’s been using is called uid. I’m going to change this to something that’s both (a) clearly private; (b) less likely to cause a naming collision. In keeping with existing Prototype convention, we’re going to call it _prototypeUID.

  • Here’s a nitpick: if (element.uid == undefined). The comparison operator (==) isn’t very precise, so if you’re testing for undefined, you should use the identity operator (===). You could also use Prototype’s Object.isUndefined. In fact, I will.

    I have a prejudice against the == operator. Most of the time the semantics of === are closer to what you mean. But this has special significance with undefined, which one encounters often in JavaScript. As an example: when you’re trying to figure out if an optional parameter was passed into a function, you’re looking for undefined. Any other value, no matter how “falsy” it is, means the parameter was given; undefined means it was not.

    (Oh, by the way: I am aware of the code screenshot on our homepage that violates the advice I just gave.)

  • There are other checks against undefined in this function. For consistency I’m going to change these to use Object.isUndefined as well. Also, the check for dflt != undefined is unnecessary: if that compound conditional passes, it means retrieve is going to return undefined anyway, so it doesn’t matter which of the two undefined values we return.

Man, I’m a bastard, aren’t I? Luckily, Element#store is similar enough that there’s no new feedback to be given here, so I’m done kvetching.

Before we rewrite this code to reflect the changes I’ve suggested, we’re going to make a couple design decisions.

Feature design

While I was deciding how to replace Element.Storage.init, I had an idea: rather than use ordinary Objects to store the data, we should be using Prototype’s Hash. In other words, we’ll create a global table of Hash objects, each one representing the custom key-value pairs for a specific element.

This isn’t just a plumbing change; it’s quite useful to be able to deal with the custom properties in a group rather than just one-by-one. And since Hash mixes in Enumerable, interesting use cases emerge: e.g., looping through all properties and acting on those that begin with a certain “namespace.”

So let’s envision a new method: Element#getStorage. Given an element, it will return the Hash object associated with that element. If there isn’t one, it can “initialize” the storage on that element, thus making Element.Storage.init unnecessary.

This new method also establishes some elegant parallels: the store and retrieve methods are really just aliases for set and get on the hash itself. Actually, retrieve will be a bit more complicated because of the “default value” feature, but we’ll be able to condense store down to two lines.

The rewrite

Enough blathering. Here’s the rewrite:

Element.Storage = {
  UID: 1

As promised, I’ve moved the UID counter. The Element.Storage object also acts as our global hashtable, but all its keys will be numeric, so the UID property won’t get in anyone’s way.

Element#getStorage assumes the duties of Element.Storage.get and Element.Storage.init, thereby making them obsolete. We’ve removed them.

  getStorage: function(element) {
    if (!(element = $(element))) return;
    if (Object.isUndefined(element._prototypeUID))
      element._prototypeUID = Element.Storage.UID++;
    var uid = element._prototypeUID;
    if (!Element.Storage[uid])
      Element.Storage[uid] = $H();
    return Element.Storage[uid];

The new getStorage method checks for the presence of _prototypeUID. If it’s not there, it gets defined on the node.

It then looks for the corresponding Hash object in Element.Storage, creating an empty Hash if there’s nothing there.

As I said before, Element#store is much simpler now:

store: function(element, key, value) {
  if (!(element = $(element))) return;
  element.getStorage().set(key, value);
  return element;

I thought about returning the stored value, to make it behave exactly like Hash#set, but some feedback from others suggested it was better to return the element itself for chaining purposes (as we do with many methods on Element).

And Element#retrieve is nearly as simple:

retrieve: function(element, key, defaultValue) {
    if (!(element = $(element))) return;
    var hash = element.getStorage(), value = hash.get(key);
    if (Object.isUndefined(value)) {
      hash.set(key, defaultValue);
      value = defaultValue;
    return value;

And we’re done.

Further refinements

In fact, we’re not done. This is roughly what the code looked like when I first checked in this feature, but some further improvements have been made.

Since we’d been using a system similar to this to associate event handlers with nodes, we had to rewrite that code to use the new storage API. In doing so, we found that we needed to include window in our storage system, since it has events of its own. Rather than define a _prototypeUID property on the global object, we give window a UID of 0 and check for it specifically in Element#getStorage.

Also, based on an excellent suggestion, we changed Element#store so that it could accept an object full of key/value pairs, much like Hash#update.

In summation

I was happy to come across Sébastien's submission. It was the perfect length for a drive-by refactoring; it made sense as a standalone piece of code, without need for an accompanying screenshot or block of HTML; and it implemented a feature we'd already had on the 1.6.1 roadmap.

You can get the bleeding-edge Prototype if you want to try out the code we wrote. Or you can grab this gist if you want to drop the new functionality in alongside

We're further grateful to Mootools for the API we're stealing. And to Wil Shipley for the recurring blog article series we're stealing.