JavaScript Sandbox Pattern

Question

Based on this Stack Overflow question I came up with the following code. I'd like your general thoughts about it. Code style, functional, logical, disadvantages, just anything you can come up based on the code that should be mentioned about it and springs into your eyes.

This is the sandbox:

function Sandbox() {
    // turning arguments into an array
    var args = Array.prototype.slice.call(arguments),
    // the last argument is the callback
    callback = args.pop(),
    // modules can be passed as an array or as individual parameters
    modules = (args[0] && "string" === typeof args[0]) ? args : args[0], i;

    // make sure the function is called
    // as a constructor
    if (!(this instanceof Sandbox)) {
        return new Sandbox(modules, args[1], callback);
    }

    // if supported use old values
    if (undefined != args[1] && "object" === typeof args[1]) {
        this.loadedScripts = args[1].loadedScripts;
        this.eventHandlers = args[1].eventHandlers;
    }

    // now add modules to the core 'this' object
    // no modules or "*" both mean "use all modules"
    if (!modules || '*' === modules) {
        modules = [];
        for (i in Sandbox.modules) {
            if (Sandbox.modules.hasOwnProperty(i)) {
                modules.push(i);
            }
        }
    }

    // always load internals
    Sandbox.modules['internal'](this);

    // initialize the required modules
    for (i = 0; i < modules.length; i += 1) {
        var module = modules[i];
        // load modules if not present
        if (typeof Sandbox.modules[module] == "undefined") {
            $.ajax({
                async : false,
                dataType : "script",
                cache : false,
                url : 'js//modules/' + module + '.js'
            });
        }
        Sandbox.modules[modules[i]](this);
    }

    // call the callback
    callback(this);

    // any prototype properties as needed
    Sandbox.prototype = {
        name : "Sandbox",
        version : "1.1",
        getName : function() {
            return this.name;
        }
    };
};

// set space for modules to add to the sandbox
Sandbox.modules = {};

Sandbox.modules.internal = function(box) {
    var box = box || {};
    box.eventHandlers = box.eventHandlers || [];
    box.loadedScripts = box.loadedScripts || [];

    box.loadScript = function(options) {
        var urls = options.urls || options.url || [];
        urls = $.isArray(urls) ? urls : [ urls ];
        $.each(urls, function(index, value) {
            if (-1 == $.inArray(value, box.loadedScripts)) {
                $.ajax({
                    async : false,
                    cache : false,
                    url : value,
                    success : function() {
                        box.loadedScripts.push(value);
                    }
                });
            }
        });
    };

    box.removeOldEventHandlers = function(options) {
        if (box.eventHandlers.length > 0) {
            for (key in box.eventHandlers) {
                if (box.eventHandlers.hasOwnProperty(key) && /^0$|^[1-9]\d*$/.test(key) && key <= 4294967294) {
                    box.eventHandlers[key].off(); // remove event handler
                    box.eventHandlers.splice(key, 1); // remove from array
                }
            }
        }
    };
};

Here a sample module ("comment.js"):

Sandbox.modules.comment = function(box) {
    var box = box || {};
    box.loadScript({
        url : "/js/template.js"
    });

    box.addComment = function(options) {
        var templateURL = options.templateURL || 'templates/comment/comment.js.tpl', commentsContainer = options.commentsContainer, trigger = options.trigger, text = options.text, guid = options.guid, newComment = $('<article style="display:none;"><img src="images/ajax-loader.gif" alt="loading comment" /></article>');

        newComment.prependTo(commentsContainer).show('fade', function() {
            $.post(SERVICE_URL, {
                o : 'comment',
                f : 'add',
                set : {
                    text : text,
                    id : id
                }
            }, function(response) {
                if (-99 == response.header.code) {
                    newComment.setTemplateURL(templateURL).processTemplate(response.content).fadeIn();
                } else {
                    box.buttonMessage({
                        message : response.header.userMessage,
                        button : trigger,
                        title : 'Error!'
                    });
                }
            }, 'json');
        });
    };
};

Here is the call to a modules function ("addComment.js"):

var Sandbox = Sandbox([ 'comment', 'message' ], Sandbox, function(box) {
    // add comment
    var trigger = $('#button_addComment'), comments = $('#comments'), commentsContainer = $('#commentsContainer', comments), input = $('#text', comments);
    box.eventHandlers.push(trigger);
    box.addComment({
        text : input.val(),
        guid : comments.attr('data-guid'),
        trigger : trigger,
        commentsContainer : commentsContainer
    });
});

And here the actual call to the code:

var SERVICE_URL = 'service.php';
$(function() {
    $('section#addComment').on('click', 'button.button', function(e) {
        e.preventDefault();
        var name= $(this).attr('id').split('_')[1];
        //getCachedScript is just the $.ajax equivalent of $.getScript but with caching
        // this would load "addComment.js"
        $.getCachedScript( "js/app/" + name + ".js" ).fail(function() {
            alert("This function is temporarily not available");
        });
    });
});

Answer 1

From a reverse once over:

• $(this).attr('id') -> could be this.id
• The click handler should not have to know where the path is of the js file
• The click handler should not have to deal with the failure of loading the js file
• The var statement in addComment.js is crazy, new lines are good for you
• Same goes your var statement in comment.js
• Using o and f as part of your API seems bad form, even for a Spartan coder like myself
• if (-99 == response.header.code) <- Why ?
• key <= 4294967294 <- This should be a commented constant
• Using for (key in box.eventHandlers) is bad form when box.eventHandlers is an array, just use the good old for loop then you probably wont even have to check for hasOwnProperty

All in all, I am not sure what you are trying to achieve with this pattern. You can download JavaScripts in an asynchronous fashion now. This will provide a much better user experience than lazy loading.

Edit:

• RE:Edge, opening a connection to the server to download less than 1kb of JavaScript will appear slower than just loading 1 extra kb of code ( I am assuming that you minify your code agressively ) upfront, especially on Edge. Loading it asynchronously means that the page will render before all the js is loaded and gives the impression of speed.
• Using booleans instead of -99 -> good idea
• Eclipse and var statements -> Maybe change editor :P

• As for the click handler, it should call an intermediate function that

  1. Knows where the js files are ("js/app/")
  2. Deals with the failure of loading the script

  Otherwise every click handler has to know the path and has to deal with script loading failure.