allphp.com

At the Edge of the Web conference in Perth last week I got to catch up with Dmitry Baranovskiy, the creator of the Raphaël and gRaphaël JavaScript libraries. Perhaps the most important thing these libraries do is make sophisticated vector graphics possible in Internet Explorer, where JavaScript performance is relatively poor. Dmitry, therefore, has little patience for poorly-written JavaScript like the code he found in Google’s just-released Closure Library.

Having delivered a talk on how to write your own JavaScript library (detailed notes) at the conference, Dmitry shared his thoughts on the new library over breakfast the next morning. “Just what the world needs—another sucky JavaScript library,” he said. When I asked him what made it ‘sucky’, he elaborated. “It’s a JavaScript library written by Java developers who clearly don’t get JavaScript.”

For the rest of the day, to anyone who would listen, Dmitry cited example after example of the terrible code he had found when he went digging through Closure. His biggest fear, he told me, was that people would switch from truly excellent JavaScript libraries like jQuery to Closure on the strength of the Google name.

“I’ll make you a deal,” I told him. “Send me some examples of this terrible code and I’ll publish it on SitePoint.”

The Slow Loop

From array.js, line 63:

for (var i = fromIndex; i < arr.length; i++) {

This for loop looks up the .length property of the array (arr) each time through the loop. Simply by setting a variable to store this number at the start of the loop, you can make the loop run much faster:

for (var i = fromIndex, ii = arr.length; i < ii; i++) {

Google’s developers seem to have figured this trick out later on in the same file. From array.js, line 153:

var l = arr.length;  // must be fixed during loop... see docs
⋮
for (var i = l - 1; i >= 0; --i) {

This loop is better in that it avoids a property lookup each time through the loop, but this particular for loop is so simple that it could be further simplified into a while loop, which will run much faster again:

var i = arr.length;
⋮
while (i--) {

But not all of Closure Library’s performance woes are due to poorly optimized loops. From dom.js, line 797:

switch (node.tagName) {
  case goog.dom.TagName.APPLET:
  case goog.dom.TagName.AREA:
  case goog.dom.TagName.BR:
  case goog.dom.TagName.COL:
  case goog.dom.TagName.FRAME:
  case goog.dom.TagName.HR:
  case goog.dom.TagName.IMG:
  case goog.dom.TagName.INPUT:
  case goog.dom.TagName.IFRAME:
  case goog.dom.TagName.ISINDEX:
  case goog.dom.TagName.LINK:
  case goog.dom.TagName.NOFRAMES:
  case goog.dom.TagName.NOSCRIPT:
  case goog.dom.TagName.META:
  case goog.dom.TagName.OBJECT:
  case goog.dom.TagName.PARAM:
  case goog.dom.TagName.SCRIPT:
  case goog.dom.TagName.STYLE:
    return false;
}
return true;

This kind of code is actually pretty common in Java, and will perform just fine there. In JavaScript, however, this switch statement will perform like a dog each and every time a developer checks if a particular HTML element is allowed to have children.

Experienced JavaScript developers know that it’s much quicker to create an object to encapsulate this logic:

var takesChildren = {}
takesChildren[goog.dom.TagName.APPLET] = 1;
takesChildren[goog.dom.TagName.AREA] = 1;
⋮

With that object set up, the function to check if a tag accepts children can run much quicker:

return !takesChildren[node.tagName];

This code can be further bulletproofed against outside interference using hasOwnProperty (see below for a full explanation of this).

return !takesChildren.hasOwnProperty(node.tagName);

If there’s one thing we expect from Google it’s a focus on performance. Heck, Google released its own browser, Google Chrome, primarily to take JavaScript performance to the next level!

Seeing code like this, one has to wonder if Google could have achieved the same thing by teaching its engineers to write better JavaScript code.

Six Months in a Leaky Boat

It would be unfair to suggest that Google has ignored performance in building Closure. In fact, the library provides a generic method for caching the results of functions that run slowly, but which will always return the same result for a given set of arguments. From memoize.js, line 39:

goog.memoize = function(f, opt_serializer) {
  var functionHash = goog.getHashCode(f);
  var serializer = opt_serializer || goog.memoize.simpleSerializer;
  
  return function() {
    // Maps the serialized list of args to the corresponding return value.
    var cache = this[goog.memoize.CACHE_PROPERTY_];
    if (!cache) {
      cache = this[goog.memoize.CACHE_PROPERTY_] = {};
    }
    var key = serializer(functionHash, arguments);
    if (!(key in cache)) {
      cache[key] = f.apply(this, arguments);
    }
    return cache[key];
  };
};

This is a clever performance trick employed in a number of major JavaScript libraries; the problem is, Google has not provided any means of limiting the size of the cache! This is fine if a cached function is only ever called with a small collection of different arguments, but this is a dangerous assumption to make in general.

Used to cache a function’s results based on, say, the coordinates of the mouse pointer, this code’s memory footprint will rapidly grow out of control, and slow the browser to a crawl.

In Dmitry’s words, “I’m not sure what this pattern is called in Java, but in JavaScript it’s called a ‘memory leak’.”

Code in a Vacuum

In his talk on building JavaScript libraries, Dmitry compared JavaScript’s global scope to a public toilet. “You can’t avoid going in there,” he said. “But try to limit your contact with surfaces when you do.”

For a general-purpose JavaScript library to be reliable, it must not only avoid interfering with any other JavaScript code that might be running alongside it, but it must also protect itself from other scripts that aren’t so polite.

From object.js, line 31:

goog.object.forEach = function(obj, f, opt_obj) {
  for (var key in obj) {
    f.call(opt_obj, obj[key], key, obj);
  }
};

for-in loops like this one are inherently dangerous in JavaScript libraries, because you never know what other JavaScript code might be running in the page, and what it might have added to JavaScript’s standard Object.prototype.

Object.prototype is the JavaScript object that contains the properties shared by all JavaScript objects. Add a new function to Object.prototype, and every JavaScript object running in the page will have that function added to it—even if it was created beforehand! Early JavaScript libraries like Prototype made a big deal of adding all sorts of convenience features to Object.prototype.

Unfortunately, unlike the built-in properties supplied by Object.prototype, custom properties added to Object.prototype will show up as an object property in any for-in loop in the page.

In short, Closure Library cannot coexist with any JavaScript code that adds features to Object.prototype.

Google could have made its code more robust by using hasOwnProperty to check each item in the for-in loop to be sure it belongs to the object itself:

goog.object.forEach = function(obj, f, opt_obj) {
  for (var key in obj) {
    if (obj.hasOwnProperty(key)) {
      f.call(opt_obj, obj[key], key, obj);
    }
  }
};

Here’s another especially fragile bit of Closure Library. From base.js, line 677:

goog.isDef = function(val) {
 return val !== undefined;
};

This function checks if a particular variable has a value defined. Or it does, unless a 3rd party script sets the global undefined variable to something else. This single line of code anywhere in the page will bring Closure Library crashing down:

var undefined = 5;

Relying on the global undefined variable is another rookie mistake for JavaScript library authors.

You might think that anyone who assigns a value to undefined deserves what they get, but the fix in this case is trivial: simply declare a local undefined variable for use within the function!

goog.isDef = function(val) {
  var undefined;
  return val !== undefined;
};

Typical Confusion

One of the most confusing aspects of JavaScript for developers coming from other languages is its system of data types. Closure Library contains plenty of bloopers that further reveal that its authors lack extensive experience with the finer points of JavaScript.

From string.js, line 97:

// We cast to String in case an argument is a Function. …
var replacement = String(arguments[i]).replace(…);

This code converts arguments[i] to a string object using the String conversion function. This is possibly the slowest way to perform such a conversion, although it would be the most obvious to many developers coming from other languages.

Much quicker is to add an empty string ("") to the value you wish to convert:

var replacement = (arguments[i] + "").replace(…);

Here’s some more string-related type confusion. From base.js, line 742:

goog.isString = function(val) {
  return typeof val == 'string';
};

JavaScript actually represents text strings in two different ways—as primitive string values, and as string objects:

var a = "I am a string!";
alert(typeof a); // Will output "string"
var b = new String("I am also a string!");
alert(typeof b); // Will output "object"

Most of the time strings are efficiently represented as primitive values (a above), but to call any of the built-in methods on a string (e.g. toLowerCase) it must first be converted to a string object (b above). JavaScript converts strings back and forth between these two representations automatically as needed. This feature is called “autoboxing”, and appears in many other languages.

Unfortunately for Google’s Java-savvy developers, Java only ever represents strings as objects. That’s my best guess for why Closure Library overlooks the second type of string in JavaScript:

var b = new String("I am also a string!");
alert(goog.isString(b)); // Will output FALSE

Here’s another example of Java-inspired type confusion. From color.js, line 633:

return [
  Math.round(factor * rgb1[0] + (1.0 - factor) * rgb2[0]),
  Math.round(factor * rgb1[1] + (1.0 - factor) * rgb2[1]),
  Math.round(factor * rgb1[2] + (1.0 - factor) * rgb2[2])
];

Those 1.0s are telling. Languages like Java represent integers (1) differently from floating point numbers (1.0). In JavaScript, however, numbers are numbers. (1 - factor) would have worked just as well.

Yet another example of JavaScript code with a whiff of Java about it can be seen in fx.js, line 465:

goog.fx.Animation.prototype.updateCoords_ = function(t) {
  this.coords = new Array(this.startPoint.length);
  for (var i = 0; i < this.startPoint.length; i++) {
    this.coords[i] = (this.endPoint[i] - this.startPoint[i]) * t +
        this.startPoint[i];
  }
};

See how they create an array on the second line?

this.coords = new Array(this.startPoint.length);

Although it is necessary in Java, it is entirely pointless to specify the length of an array ahead of time in JavaScript. It would make just as much sense to create a new variable for storing numbers with var i = new Number(0); instead of var i = 0;.

Rather, you can just set up an empty array and allow it to grow as you fill it in. Not only is the code shorter, but it runs faster too:

this.coords = [];

Oh, and did you spot yet another inefficient for loop in that function?

API Design

If all the low-level code quality nitpicks above don’t convince you, I defy you to try using some of the APIs Google has built into Closure Library.

Closure’s graphics classes, for example, are modeled around the HTML5 canvas API, which is about what you’d expect from a JavaScript API designed by an HTML standards body. In short, it’s repetitive, inefficient, and downright unpleasant to code against.

As the author of Raphaël and gRaphaël, Dmitry has plenty of experience designing usable JavaScript APIs. If you want to grasp the full horror of the canvas API (and by extension, Closure’s graphics API), check out the audio and slides from Dmitry’s Web Directions South 2009 talk on the subject.

Google’s Responsibility to Code Quality

By this point I hope you’re convinced that Closure Library is not a shining example of the best JavaScript code the Web has to offer. If you’re looking for that, might I recommend more established players like jQuery?

But you might be thinking “So what? Google can release crappy code if it wants to—nobody’s forcing you to use it.” And if this were a personal project released by some googler on the side under his or her own name, I’d agree with you, but Google has endorsed Closure Library by stamping it with the Google brand.

The truth is, developers will switch to Closure because it bears the Google name, and that’s the real tragedy here. Like it or not, Google is a trusted name in the development community, and it has a responsibility to that community to do a little homework before deciding a library like Closure deserves public exposure.

Related Posts

1. Google Releases its JavaScript Closure Tools
2. Serializing PHP data structures for Javascript
3. How to Write a Cookie-less Session Library for JavaScript