As it currently stands, this question is not a good fit for our Q&A format. We expect answers to be supported by facts, references, or expertise, but this question will likely solicit debate, arguments, polling, or extended discussion. If you feel that this question can be improved and possibly reopened, visit the help center for guidance. Closed 10 years ago.

NOTE: Updated and rewritten

This question has been redone and updated. Please pardon outdated references below. Thanks.

I've seen a lot of javascript code lately that looks wrong to me. What should I suggest as a better code pattern in this situation? I'll reproduce the code that I've seen and a short description for each one:

Code block #1

This code should never evaluate the inner function. Programmers will be confused because the code should run.

$(document).ready( function() { 
  return function() { 
    /* NOPs */
  }
});

Code block #2

The programmer likely intends to implement a self-invoking function. They didn't completely finish the implementation (they're missing a () at the end of the nested paren. Additionally, because they aren't doing anything in the outer function, the nested self-invoking function could be just inlined into the outer function definition.

Actually, I don't know that they intend a self invoking function, because the code is still wrong. But it appears they want a self invoking function.

$(document).ready( (function() { 
  return function() { 
    /* NOPs */
  }
}));

Code block #3

Again it appears the programmer is trying to use a self-invoking function. However, in this case it is overkill.

$(document).ready( function() { 
  (return function() { 
    /* NOPs */
  })()
});

Code block #4

an example code block

$('#mySelector').click( function(event) { 
  alert( $(this).attr('id') );

  return function() { 
    // before you run it, what's the value here?
    alert( $(this).attr('id') );
  }
});

Commentary:

I guess I'm just frustrated because it causes creep bugs that people don't understand, changes scoping that they're not grokking, and generally makes for really weird code. Is this all coming from some set of tutorials somewhere? If we're going to teach people how to write code, can we teach them the right way?

What would you suggest as an accurate tutorial to explain to them why the code they're using is incorrect? What pattern would you suggest they learn instead?

All the samples I've seen that have caused me to ask this question have been on SO as questions. Here's the latest particular snippet I've come across that exhibits this behavior. You'll notice that I'm not posting a link to the question, since the user appears to be quite the novice.

$(document).ready(function() {
 $('body').click((function(){
  return function()
  {
   if (counter == null) {
    var counter = 1;
   }
   if(counter == 3) {
     $(this).css("background-image","url(3.jpg)");
     $(this).css("background-position","10% 35%");
     var counter = null;
   }
   if(counter == 2) {
     $(this).css("background-image","url(2.jpg)");
     $(this).css("background-position","10% 35%");
     var cou开发者_Python百科nter = 3;
   }
   if(counter == 1) {
     $(this).css("background-image","url(1.jpg)");
     $(this).css("background-position","40% 35%");
     var counter = 2;
   }


  }
 })());
});

Here's how I proposed that they rewrite their code:

var counter = 1;
$(document).ready(function() {
    $('body').click(function() {
        if (counter == null) {
            counter = 1;
        }
        if (counter == 3) {
            $(this).css("background-image", "url(3.jpg)");
            $(this).css("background-position", "10% 35%");
            counter = 1;
        }
        if (counter == 2) {
            $(this).css("background-image", "url(2.jpg)");
            $(this).css("background-position", "10% 35%");
            counter = 3;
        }
        if (counter == 1) {
            $(this).css("background-image", "url(1.jpg)");
            $(this).css("background-position", "40% 35%");
            counter = 2;
        }
    });
});

Notice that I'm not actually saying my code is better in any way. I'm only removing the anonymous intermediary function. I actually know why this code doesn't originally do what they want, and I'm not in the business of rewriting everyone's code that comes along, but I did want the chap to at least have usable code.

I thought that a for real code sample would be appreciated. If you really want the link for this particular question, gmail me at this nick. He got several really good answers, of which mine was at best mid-grade.

Your first example is strange. I'm not even sure if that would work the way the author likely intended it. The second simply wraps the first in unnecessary parens. The third makes use of a self-invoking function, though since the anonymous function creates its own scope (and possibly a closure) anyways, I'm not sure what good it does (unless the author specified additional variables within the closure - read on).

A self-invoking function takes the pattern (function f () { /* do stuff */ }()) and is evaluated immediately, rather than when it is invoked. So something like this:

var checkReady = (function () {
    var ready = false;
    return {
        loaded: function () { ready = true; },
        test: function () { return ready; }
    };
}())
$(document).ready(checkReady.loaded);

creates a closure binding the object returned as checkready to the variable ready (which is hidden from everything outside the closure). This would then allow you to check whether the document has loaded (according to jQuery) by calling checkReady.test(). This is a very powerful pattern and has a lot of legitimate uses (namespacing, memoization, metaprogramming), but isn't really necessary in your example.

EDIT: Argh, I misunderstood your question. Didn't realize you were calling out poor practices rather than asking for clarification on patterns. More to the point on the final form you asked about:

(function () { /* woohoo */ }())
(function () { /* woohoo */ })()
function () { /* woohoo */ }()

evaluate to roughly the same thing - but the first form is the least likely to have any unintended consequences.

Anonymous functions are useful for:

Passing logic to another function
Declaring single use functions
Declaring a function without adding a variable to the scope
Providing scope for variables
Dynamic programming

You can read more on these topics at the following link: javascript anonymous functions

I'm not sure if you are recreating your examples exactly or not, but for this

// I picked document ready from jQuery cos you ALL know it, I'm sure ;) 
$('#mySelector').click( function(event) { // this line is all boilerplate, right? 
  alert( $(this).attr('id') ); 

  return function() { // <-- WHAT THE ???? IS THIS ????  ? 
    // before you run it, what's the value here? 
    alert( $(this).attr('id') ); 
  } 
});

as well as your first example, the returned anon function never runs. if you were returning false here instead of the function, the default action would be prevented. as is I guess it gets evaluated as event handler returned truish by the browser.

if it were changed to this

// I picked document ready from jQuery cos you ALL know it, I'm sure ;) 
$('#mySelector').click( function(event) { // this line is all boilerplate, right? 
  alert( $(this).attr('id') ); 

  return function() { // <-- WHAT THE ???? IS THIS ????  ? 
    // before you run it, what's the value here? 
    alert( $(this).attr('id') ); 
  } 
}() );

the outer function would execute at the time where we're attaching the event handler. the first alert would happen at that time as well as any other preparation taking place in the same scope. the result is to produce the actual method fired during 'onclick', which is the inner function which also has access to the data during the prep phase.

I can see this being appropriate in certain cases although it seems rather convoluted. As for the former example, I'll just assume your programmer has no idea what they are doing.

Code block #3 is the only one that is even remotely sane - you don't return a function from an event handler, ever. Self-executing anonymous functions, however, have many uses, like namspacing or private variables. Most of those uses are unnecessary inside an event handler (which is an anonymous function itself), though. Still, they can be used to avoid unwanted side effects of closures:

$(document).ready(function() {
    for (var i = 9; i <= 10; i++) {
        $('#button'+i).click(function() {
            alert('Button '+i+' clicked');
        });
    }
});

this will alert "Button 10 clicked" for all the buttons, but this one will work correctly:

$(document).ready(function() {
    for (var i = 9; i <= 10; i++) {
        (function() {
            var j = i;
            $('#button'+j).click(function() {
                alert('Button '+j+' clicked');
            });
        })();
    }
});

I think a lot of people will agree with your sentiment that sometimes its not good to try and be too cute. If you don't need a language construct, then dont use it, especially if it is confusing.

That said, sometimes you do need these things.

(function(){
    var x = 'test';
    alert(x);
})()

alert(typeof(x)); //undefined

Here, the x variable is locked into the functions scope.

(I found the above example at http://helephant.com/2008/08/javascript-anonymous-functions/)

So, you can do weird things with scope with javascript; this is one way. I agree with your sentiment that a lot js code is more complicated than it needs to be.

They all appear to be forms of javascript closure's and if you have asynchronous calls happening, they can often be the only way to have the correct value of a variable in the proper scope, when the function finally gets called.

For examples/explanations, see:

How do JavaScript closures work?
http://blog.morrisjohns.com/javascript_closures_for_dummies.html
http://www.bennadel.com/blog/1482-A-Graphical-Explanation-Of-Javascript-Closures-In-A-jQuery-Context.htm

For code block one, that looks a little like a pattern I've seen used for objects with private members. Consider:

function MyObject()  {
   var privateMember = 1;

   return {
        "Add":function(x) {return x + privateMember;},
        "SetPrivateMember":function(x) {privateMember = x;},
        "GetPrivateMember":privateMember
   };
}