Signs of a poorly written jQuery plugin

转载：http://remysharp.com/2010/06/03/signs-of-a-poorly-written-jquery-plugin/

So far with every single workshop I’ve given, both for advanced JavaScript and jQuery for Designers, this question (or some variation thereof) has come up:

How do you know if the plugin is good to use?

It’s always dependant on the problem they’re trying to solve, but in lieu of a better jQuery plugin ranking system, here’s a couple of tips that should raise a red flag.

Consider the following:

$.fn.myplugin =function(){
  var me = $(this).each(function(){
    return $(this).bind('someEvent',function(){
      // does something
    });
  });

  return me;
};

Although the code may be perfect once some event has run, most times you don’t have time to read through all the code carefully and you need to make a decision so you can move on to the actual problem you’re trying to solve.

In the code above, there’s a number of red flags that have gone up for me, and I tend to look in this area of code first. If these patterns have been used, it tells me the author hasn’t quite grasped how jQuery works and hasn’t considered making simple tuning changes.

The inline return

$.fn.myplugin =function(){
  var me = $(this).each(fn);
  return me;
};

Should be written as:

$.fn.myplugin =function(){
  return $(this).each(fn);
};

The me variable isn’t being used again, so there’s no point in creating it.

Double jQuery

$.fn.myplugin =function(){
  return $(this).each(fn);
};

Whilst within the context of the plugin code – i.e. within the function attached to .fn, the keyword this refers to the jQuery instance, not DOM elements.

If I were to rewrite this to show you the value, you’d see:

$.fn.myplugin =function(){
  return $($('div.foo')).each(fn);
};

So within the actual plugin (not jQuery callbacks), this refers to jQuery, so we can access jQuery’s methods directly:

$.fn.myplugin =function(){
  returnthis.each(fn);
};

Returning what to each?

$.fn.myplugin =function(){
  returnthis.each(function(){
    return $(this).bind('someEvent', fn);
  });
};

jQuery’s each iterator simply loops, it doesn’t collect anything. The result variable is jQuery with the original collection inside it still – you can’t modify the collection by returning or not returning.

So return isn’t required at all in this case:

$.fn.myplugin =function(){
  returnthis.each(function(){
    $(this).bind('someEvent', fn);
  });
};

Wasteful use of each

$.fn.myplugin =function(){
  returnthis.each(function(){
    $(this).bind('someEvent', fn);
  });
};

Hopefully by removing all the cruft from the starting version, this next step should be obvious. If not, here’s a clue:

• What’s returned from an each call? A jQuery collection.
• What’s returned from a bind call? A jQuery collection.

Since we’re running the bind on each element, and only doing that, it means there’s no difference. So let’s throw away the each call and just return the bind:

$.fn.myplugin =function(){
  returnthis.bind('someEvent', fn);
};

Remember that within the plugin, this refers to the jQuery instance, and not the element, so we don’t need the wrapping $().

All better now, eh?