JavaScript dynamic <script> creation

I've been focusing on my PHP skills lately but have been shifting to JavaScript. I'm familiar with the bare-bone basics of jQuery. I'm not as familiar with JavaScript as I'd like to be. I'm a solo-developer so I'd just like somebody to take a look at this and point out any mistakes or things I could be doing better.

What the code does

Creates an arbitrary number of ` elements, assigns a type and src attribute to each one and then inserts that element before the .

The code

See Better Code below!

function async_init() {

    var element, type, src;
    var parent = document.getElementsByTagName('body');
    var cdn = new Array;

    cdn[0] = 'http://code.jquery.com/jquery-1.6.2.js';
    cdn[1] = 'https://ajax.googleapis.com/ajax/libs/jqueryui/1.8.14/jquery-ui.js';

    for (var i in cdn) {
        element = document.createElement('script');
        type    = document.createAttribute('type');
        src     = document.createAttribute('src');

        type.nodeValue = 'text/javascript';
        src.nodeValue  = cdn[i];

        element.setAttributeNode(type);
        element.setAttributeNode(src);
        document.body.insertBefore(element, parent);
    }

}

Right now the code is working, which is great. But my questions:

• Is this the "best" way to do this? I define best as optimum user experience and efficient code.

• Are there any obvious "noobie" flaws in my JavaScript?

• Is there anyway I could get the onload attribute out of the body?




I used Mozilla Developer Network as my JavaScript reference. If there are any other references that are accurate, documented & useful I would love to have more information.

Thanks for checking the code out and your feedback, even if its pointing out how my code sucks!

Better code after critique

``
function async_init() {
var element;
var cdn = new Array;

cdn[0] = 'http://code.jquery.com/jquery-1.6.2.js';
cdn[1] = 'https://ajax.googleapis.com/ajax/libs/jqueryui/1.8.14/jque

You've erred many times in the above code. However, that means you get to learn a lot about how to properly interact with the DOM. In many instances, there are built-ins that quickly and efficiently get the job done.

First off, you're incorrectly accessing the body. document.body is a well-supported reference to the body that's been around since at least DOM Level 1.

Secondly, you're taking the hard route towards creating an array. In his book, "JavaScript: The Good Parts", Douglas Crockford insists on using literals instead of constructors. In this case, an array literal is []. This is an easier way of calling new Array().

Thirdly, you're incorrectly iterating over the cdn array with a for...in loop instead of a for loop. A for...in loop is meant for objects as it iterates over the properties of an object. Not only is it slower than a normal for loop, but it also is very susceptible to prototype modification. For example, try running this code with a build of MooTools (jsFiddle has one enabled by default):

for(var key in [])
{
    console.log(key);   
}

You'll get a ton of hits because MooTools modifies the Array prototype. You'd have to do a hasOwnProperty check for each property just to avoid those pitfalls. Avoid for...in on anything but objects.

Next, declaring the i variable inside of the for...in loop gives the illusion of block scope. This isn't the case, since i is available anywhere in the function after it's been defined. Only functions have scope in JavaScript. Here are Crockford's thoughts via "...The Good Parts" (page 102):


In most languages, it is generally best to declare variables at the first site of use. That turns out to be a bad practice in JavaScript because it does not have block scope. It is better to declare all variables at the top of each function.

Finally, setting the attributes for each script tag isn't necessary. Since you're already interacting with the DOM, setting their cousin, the DOM property is a better idea. The DOM provides shortcuts to node properties and is more stable than setting attributes. get/set/removeAttribute all have the unfortunate shortcoming of being very weird in various Internet Explorer builds. They're best avoided unless completely necessary.

Fixed + optimized:

function async_init() {
    var element;
    var parent = document.body;
    var cdn = ["http://code.jquery.com/jquery-1.6.2.js", "https://ajax.googleapis.com/ajax/libs/jqueryui/1.8.14/jquery-ui.js"];
    var i = 0, file;
    for (i;i<cdn.length;i++) {
        file = cdn[i];
        element = document.createElement("script");
        element.type = "text/javascript";
        element.src  = file;
        parent.appendChild(element);
        //free file's value
        file = null;
    }
    /*
    * Cleanup at end of function, which isn't necessary unless lots of memory is being         
    * used up.
    * No point nulling a Number, however. In some ECMAScript implementations
    * (ActionScript 3), this throws a warning.
    */
    //empty array
    cdn.splice(0, cdn.length);
    //clear variable values
    element = null;
    parent = null;
    cdn = null;
}

Also note that since you're adding these scripts asynchronously, there's no stable way to detect when the file has loaded, short of a script loader or a timer (my opinion is neither are stable).

An alternative is to use document.write to append these scripts synchonously. However, you'll have to use a DOMString and escape the forward slash in the script's end tag. This will append it to the end of the body tag.

Example: document.write("");

Note that the backslash character (\) escapes characters in JavaScript. I've used it to escape the double quotes for each attribute along with the forward slash in the end tag.

If you have any more questions regarding JavaScript or the DOM, feel free to ask in the comments.

-Matt

StackExchange Code Review Q#3274, answer score: 5