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 counter = 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.