# Does this function have to be nested?

Recent JavaScript student here. I ran afoul of some weirdness with event listeners that led to my code being unresponsive. After some reading and tweaking, I arrived at a version that worked in JSFiddle, but not in the browser. I finally got it working in the browser, but now my code looks like this:

window.onload = function () {
}
}


But the use of window.onload and nesting the actual useful code in an anonymous function seems bloated/hacky to me. What, if anything, can I do to clean it up?

Available on a JSFiddle here.

-

window.onload = function () {


Mixing DOM-0 event handlers and addEventListener is inconsistent, pick one and stick with it.

window.addEventListener("load", function () {


function bgnAddChllng() {


Don't disemvowel your variables. This isn't Wheel of Fortune, vowels don't cost anything.

function beginAdditionChallenge() {


var addChllng = document.getElementById("addChllng");


Use proper indentation. JSFiddle has a handy TidyUp button to do it for you. Consider a more descriptive ID for the button.

var button = document.getElementById("challengeButton");


addChllng.addEventListener('click', bgnAddChllng, false);


Switching between single and double quotes is inconsistent, pick one and stick with it. useCapture defaults to false, there's no need to explicitly pass it.

button.addEventListener("click", beginAdditionChallenge);


nesting the actual useful code in an anonymous function seems bloated/hacky to me.

It's a pretty common practice, but if you don't like it, give the function a name and don't nest it, just as you did with the button click event handler.

function beginAdditionChallenge() {
}

function initChallenge() {
var button = document.getElementById("challengeButton");
}


-
Wow, that's very thorough, specific, and helpful. Thank you! My only question in response to all that is about the last bit--between the nesting option and the non-nesting option you provided, which would you recommend? The nested syntax seems odd to me, but I also have less than 10 hours of Javascript experience, so my perception is quite likely groundless. – user242007 Jun 14 '14 at 7:49
@user242007 I actually prefer to avoid anonymous, nested functions when possible; it keeps the code neater (less indentation) and gives more readable stack traces when errors occur. – Dagg Jun 14 '14 at 7:56
@user242007 do keep in mind that doing it this way pollutes the global scope, though. I'm assuming that at some point all of this will end up nested within another function (scope) to keep things out of the global scope. – Dagg Jun 14 '14 at 7:59
Good point, I guess it shows my inexperience that the scoping issue hadn't even crossed my mind. Luckily, you've addressed it before I've done any more work on the project. You're a real life-saver. – user242007 Jun 14 '14 at 8:10
@dagg Nice answer - much more thorough than my own. And also good point (in the comments) about reducing indentation by favoring named functions. The other benefit of named functions is that they encourage writing reusable functions. As for keeping the global scope clean, wrapping everything ub an IIFE fixes that handily. 1 extra level of indentation, sure, but that's a fair trade, I think. – Flambino Jun 14 '14 at 12:36

There's nothing really wrong with your code (apart from some missing indentation). The structure could be different, but functions are first-class objects; you shouldn't worry about declaring functions any more than you might about declaring any other variable.

Besides, your function isn't really anonymous: You're assigning it to window.load. There's little difference between saying var x = function () {...} and function x() {...}

However, you should stick to one event API; right now you're using both window.load and addEventListener. The latter is the better choice.

window.addEventListener("load", function (event) {
...
}, false);

-

If you place your code in a script tag at the end of the body, right before the </body> tag, then the code can just be this:

document.getElementById('addChllng').addEventListener('click', function() {
});


If you place your code before the addChllng element, then you will have to use some sort of system for detecting when the document is ready before you can get the addChllng element. window.onload is one such way (though it waits for all images to load which isn't necessary). Using window.onload, you could have just this;

window.addEventListener('load', function() {
});
});


If you a bit more extensible system that won't wait until all images are loaded and can be used multiple times for different pieces of code in your project and you aren't using a framework that already has this built into it, then you can use the docReady() function in this answer.

-