views:

76

answers:

3

I'm creating a jQuery plugin. So far it's working fine, but I'm having doubt about the way I'm doing things:

jQuery.fn.myMethod = function() {
  return this.each(function(){
    MyScope.doSomething(jQuery(this).attr("id"));
  });
};

var MyScope = {

  // The functions contained in MyScope are extremely linked to the logic 
  // of this plugin and it wouldn't make a lot of sense to extract them

  doSomething: function(id){
    // something
    doSomethingElse(23);
    // some more code
    doSomethingElse(55);
  },

  doSomethingElse: function(someInt){
    // some code 
  }
};

I use MyScope to store all my "private" functions. I don't want the user to be able to go $("p").doSomething(), but I do need to use them.

I could move everything in the myMethod function, but it would create a 100 lines long function and people would hate me for it.

What's the best practices in this situation? Are there any great tutorials out there regarding this?

+5  A: 

You can encapsulate your functions to do what you want, like this:

jQuery.fn.myMethod = function() {
  return this.each(function(){
    doSomething(jQuery(this).attr("id"));
  });        
  function doSomething(id){
    //do something
  }
  function doSomethingElse(){
    // some code
  }
};

You can view a quick demo here

"I could move everything in the myMethod function, but it would create a 100 lines long function and people would hate me for it." ....why?

The code has to be defined somewhere, if you don't want it to be externally accessible there are a few ways, but I don't see why anyone would dislike you doing exactly this. It's all about scope and where you want things, as long as you're not declaring it multiple times and exposing only what you want, I don't see any problem.

There are several styles to declaring it, some with the same effect, the option I gave is one of many, but placing things inside myMethod is a perfectly reasonable approach.


To be more complete, here's another alternative:

(function($) { 
    function doSomething(id){
      //do something, e.g:  doSomethingElse('bob');
    }
    function doSomethingElse(str){
      //some code
    }
    $.fn.myMethod = function() {
      return this.each(function(){
        doSomething(jQuery(this).attr("id"));
      });   
    };
})(jQuery);

Or another:

(function($) { 
    var me = {
        doSomething: function(id){
         //do something, e.g:  this.doSomethingElse('bob');
        },
        doSomethingElse: function(str){
          //some code
        }
    };
    $.fn.myMethod = function() {
      return this.each(function(){
        me.doSomething(jQuery(this).attr("id"));
      });   
    };
})(jQuery);

Related articles:

Nick Craver
@nick: having only one huge function and 0 code reuse is fairly bad imho. Especially since I call doSomethingElse() in 3 or 4 different places in doSomething().
marcgg
@marcgg: Your example doesn't show that...you should give more clues about your usage if that's the case, even *hinting* these will be used for *multiple* plugins would be a start.
Nick Craver
@nick I'll update my example right now
marcgg
@marcgg: That would help in getting a better answer :)...one plugin method is the norm, multiple methods isn't, so with no other hints, people will assume what you're doing matches the majority of cases.
Nick Craver
@nick: yeah I realize now. It's updated.
marcgg
@marcgg: Your updated example still fits, for example: http://jsfiddle.net/CcxmM/ You're not declaring any code twice, unless I'm still misunderstanding your overall plugin?
Nick Craver
@nick: placing everything in myMethod() won't imply that the functions will be re-defined each time the $().myMethod() is called? Because if it's does I'm kinda worried about the performances
marcgg
@nick Yeah the code you put on fiddle would work! (thanks ^^) I'm just worried a bit about performances
marcgg
@marcgg: Nope it won't...you're not re-defining the function, you're just passing a value in and doing something with it (in this case you're using `this` instead of a value, but no different). No copy of the function is made, it's re-used as it should be. I'd checkout the closure discussion here: http://stackoverflow.com/questions/111102/how-does-a-javascript-closure-work
Nick Craver
@nick: What I mean is that since the definition of doSomething is in myMethod, it seems to me that every time I call myMethod it will redefine doSomething reducing performances compared to a solution like Time Down's... isn't it?
marcgg
(reading the post on closures, maybe I missed something)
marcgg
@marcgg - I'm actually not sure how to explain this clearly...when you call a function you're not making a copy, you're just going through it. If a function had internal variables that might change per run of the function you'd need a "copy", but since JavaScript's single-threaded, there's no need to do that. Also remember that in every modern browser your code will be optimized way beyond what you are able to do in the JS anyway.
Nick Craver
Nick, marcgg is right: you'll get a new function object for `doSomething` each time `myMethod` is executed.
Tim Down
Aha, who's right? I just read that "a closure is a stack-frame which is not deallocated when the function returns" so it kinda makes me think that @nick is right, but now @tim is making me doubt
marcgg
It's easy to check. `var f = function() { function g() {}; return g; }; var g1 = f(), g2 = f(); alert(g1 === g2);` If Nick is right, you'd expect `true`, if I'm correct you'd expect `false`. Guess which it is :)
Tim Down
@Tim Down: In that case you're returning it outside the closure it's in, so of course it'd need a copy each time to expose it, I'm not sure that's a valid test at all. To be clear, not saying you're wrong, just saying that test isn't a valid way to determine it.
Nick Craver
@marcgg: I can't think of a valid way to test this, but then again I don't think it matters...this will be optimized by the browser in other ways (the tracing engines do *incredible* amounts of optimization under the covers). @Tim may be right in terms of what *should* happen, to be safe use the other 2 approaches I added to the answer...but what the spec says happens, and what actually happens from a performance standpoint (while still *behaving* like the spec) are two different things, just be aware of that.
Nick Craver
Nick: Actually, I should maybe have started with the ECMAScript 3 specification, in particular section 10 on execution contexts. Some choice phrases: "Every function and constructor call enters a new execution context" (section 10.2), which involves going through the variable instantiation phase. As part of that, "For each FunctionDeclaration in the code, in source text order, create a property of the variable object whose name is the Identifier in the FunctionDeclaration, whose value is the result returned by creating a Function object as described in 13" (from section 10.1.3).
Tim Down
@Tim: refer to my previous comment :) I'm not disagreeing with the spec, I'm saying what gets in-lined, traced and optimized make it a non-issue, though not in older browsers.
Nick Craver
I saw your comment before posting mine but by then I was so far into my spec pedantry I couldn't possibly stop :) You may well be right that it could be optimized by the browser, and it may well not matter on anything other than a large scale, but it's still worth knowing.
Tim Down
@Tim - Completely agree, and using one of the other methods added ensures it's a non-issue in all cases, definitely useful notes all around.
Nick Craver
Ok so... what answer to accept ^^ ? I guess Nick's since it's the most upvoted and with a lot of different cases. If you could just add some reference to what we said in the comments + links and that'd be a pretty cool SO entry. I certainly learned a lot
marcgg
Drawing people's attention to this answer is probably more useful than drawing them to mine because of the discussion, so I'd say this one. Shame there's no points for assists :)
Tim Down
@Tim - you earned an assist point from me :)
Nick Craver
+2  A: 

I would put everything inside the jQuery.fn.myMethod function, to avoid possible namespace collisions, and make for cleaner code. This pattern also allows you to to make private methods which are not accessible from outside the myMethod function.

jQuery.fn.myMethod = function(options) {
  // Set up a "that" object, which can be referenced from anywhere inside this function
  var that = {};

  // If the plugin needs optional arguments, you can define them this way
  if (typeof(options) == 'undefined') options = {};
  that.options.option1 = options.option1 || 'default value 1';
  that.options.option2 = options.option2 || 'default value 2';

  that.init = function() {
    // psuedo-constructor method, called from end of function definition
  }

  that.doSomething = function() {
    // something
  }

  that.doSomethingElse = function() {
    // something else
  } 

  // Call init method once all methods are defined
  that.init();

  // Return the matched elements, to allow method chaining
  return jQuery(this);
}
Harold1983-
that is pretty interesting, do you know if it's a common practice?
marcgg
What's the point of `that`? You could use local variables instead of an object with properties.
Tim Down
I think so, at least among my colleagues and I. I've seen plenty of jQuery plugins written this way also, or at least some variation of this.
Harold1983-
The point of the "that" object is to avoid possible namespace collisions. For example, if I had just called my methods init(), doSomething(), and doSomethingElse(), I risk accidentally calling a global function by the same name. While this is unlikely, it's still one less thing to worry about. Also, you can pass the "that" object to callback methods, giving them access to the current scope.
Harold1983-
@harold1983: any example out there?
marcgg
@Harold1983-: While the global thing is true, you'd be using the local-est reference...I *completely* agree it's best practice to avoid that, but it wouldn't actually be an issue, at least not in any case I can think of.
Nick Craver
Harold: OK, those points are valid in general, but don't apply to that example. Fair enough.
Tim Down
+3  A: 

There's nothing wrong with using a large function just to create a new scope. The following keeps doSomething and doSomethingElse private and avoids defining new doSomething and doSomethingElse functions for each invocation of myMethod, which is what would happen if you put doSomething and doSomethingElse inside myMethod's definition.

(function() {
  function doSomething(id) {
    // Something
  }

  function doSomethingElse() {
    // Something else
  }

  jQuery.fn.myMethod = function() {
    return this.each(function(){
      doSomething(this.id);
    });
  };
})();
Tim Down
Sorry but I'm not sure what you are trying to do here. Could you give an example of how this would be used?
marcgg
The point is that `doSomething` and `doSomethingElse` are only accessible within the enclosing function, not outside. The example is the call to `doSomething(this.id);` within the definition of `jQuery.fn.myMethod`. Keeping the `doSomething` and `doSomethingElse` functions from being accessible to other code is what I thought you wanted and is what this achieves. Have I misunderstood the question?
Tim Down
@tim: That's pretty cool, thanks! I'm not really familiar with this ( function(){})(); syntax, is there any documentation on what this will do?
marcgg
@marcgg: http://blog.morrisjohns.com/javascript_closures_for_dummies.html No offense meant by the name, but it's a good closure learning reference.
Nick Craver
marcgg: It's commonly referred to as the Module Pattern. If you Google for it, there's plenty of references. This one looks reasonable: http://www.adequatelygood.com/2010/3/JavaScript-Module-Pattern-In-Depth
Tim Down
@nick, @tim: reading...
marcgg