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");
 });
 });
});

• elclanrs: In box.loadScript you can reduce to var urls = [].concat(options.url||options.urls||[]) and you save the line below.
• steros: Thanks! What I do not like so far is in "addComment.js". As you can see there are two modules loaded comment and message. But message is actually needed in the module comment for the function box.buttonMessage. So actually that module should be loaded in the module comment! But today I did not figure out how to do this. I guess I need to create a function for the code of loading modules in the Sandbox itself and make this function callable in a module to load (an)other module(s).
• 200_success: What you may and may not do after receiving answers. I've rolled back Rev 6 → 5.

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.

• steros: Thanks for your answer! Actually I want to separate code and implement lazy loading to improve page speed. Why load all the magicTM if you never make a comment? I'm not sure how asynchronous loading would help in all cases. Imagine if you are on Edge on a mobile phone.
• steros: For the click handler I think I either don't understand what you mean or I have no idea how to do it in another way. My var statement is usually chained but to different lines, to this day I could not find out how to tell eclipse not to push it back into one line.
• steros: if(-99 == response.header.code) is a bit historic. code is -99 if everything is fine and a number bigger than zero if not (error codes). You are right maybe I should change it to if(true == response.header.success) and elseif(true == response.header.failure)? Something like that or do you have a better option?
• konijn: Update question.