Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I have written a JavaScript API that loads and houses libraries of code to do all sorts of things on websites.

Effectively, this is (will be) the base JavaScript for all the websites I build.

The aim of this API is to load the required libraries (SM.load()) needed for a particular page, and add them to the library as a plugin (SM.run()).

You may be surprised as to how I have written this library, just to explain in brief. I have aliased the jQuery selector, so that if you run:

SM('#test').foo()

It will retrieve #test using the jQuery constructor function and then chain the result through to the foo() function. However, you can also run functions like this:

SM.bar()

Effectively this just mimics the way you can do this in jQuery:

$('#test').show()
$.ajax(options)

/**
 * Sine Macula Javascript API
 * The Sine Macula API contains all base functions for the Sine Macula
 * Javascript Library
 * @name class.sinemacula.js
 * @author Ben Carey
 * @version 1.0
 * @date 20/11/2012
 * @copyright (c) 2012 Sine Macula Limited (sinemaculammviii.com)
 */
(function(global, $){

    // Enable strict mode
    "use strict";

    // Set the global variables for Sine Macula
    var oldSM, SM;

    // Make sure jQuery has loaded
    if(typeof $ !== "function"){
        throw "jQuery must be loaded";
    }

    /**
     * Sine Macula Object
     * The main Sine Macula library
     *
     * @param string The element to be selected
     */
    SM = global.SM = global.SineMacula = SineMacula;
    function SineMacula(selector){
        // Construct and return an instance of the
        // Sine Macula object
        return new _initialize(selector);
    }

    /**
     * Sine Macula Constructor
     * Alias of the jQuery Selector
     *
     * @param string The element to be selected
     */
    function _initialize(selector){
        // Retrieve the elements matching the selector
        // and make it available to all the Sine Macula
        // methods
        this.elements = $(selector);
    }

    // So that plugins can add to _initialize.prototype, make it available
    // as a property of SineMacula: SineMacula.fn (aka SM.fn)
    SM.fn = _initialize.prototype;

    /**
     * Sine Macula Run
     * Makes it easy to write plugins for the Sine Macula library
     *
     * @param function callback
     */
    SM.run = run;
    function run(callback){
        // Call the function with the Sine Macula
        // and jQuery objects
        callback(SM, $);
    }

    /**
     * Sine Macula Load
     * Load the Sine Macula Libraries and Plugins
     * into the current document
     *
     * The options:
     * - package: the package of libraries to load
     * - packageURL: a remote source to load the package details from
     * - libraries: any additional libraries to load
     *
     * @param object options The options for the Sine Macula load
     */
    SM.load = load;
    function load(options){     
        var url,query,script;       
        // Set the defaults for the loader
        var options = $.extend({
            package:    'none', // Do not load any packages by default
            packageURL: false, // Do not retrieve the package details from a URL by default
            libraries:  [] // Do not load any libraries by default
        },options);     
        // Build the query based on the parameters supplied
        if(options.packageURL){         
            // Build the query to allow for a remote
            // package definition
            query = '?packageURL='+encodeURIComponent(options.packageURL);
        }else if(options.package=='none'){          
            // If no package has been supplied then just
            // provide libraries to load
            query = '?libraries='+encodeURIComponent(options.libraries.join());
        }else{          
            // If a package has been supplied then
            // request it, and any additional libraries
            query = encodeURIComponent(options.package)+'/?libraries='+encodeURIComponent(options.libraries.join());
        }       
        // Complete the url by appending the query
        url = '//libraries.sinemaculammviii.com/'+query;        
        // Append the script tag to the end of the document
        script = document.createElement('script');
        script.type = 'text/javascript';
        script.src = url;
        $('head')[0].appendChild(script);       
    }

    /**
     * IndexOf
     * A fix to allow the call of the indexOf function
     * in Internet Explorer
     */
    if(!Array.prototype.indexOf){
        Array.prototype.indexOf = function (searchElement /*, fromIndex */ ) {
            "use strict";
            if (this == null) {
                throw new TypeError();
            }
            var t = Object(this);
            var len = t.length >>> 0;
            if (len === 0) {
                return -1;
            }
            var n = 0;
            if (arguments.length > 1) {
                n = Number(arguments[1]);
                if (n != n) { // shortcut for verifying if it's NaN
                    n = 0;
                } else if (n != 0 && n != Infinity && n != -Infinity) {
                    n = (n > 0 || -1) * Math.floor(Math.abs(n));
                }
            }
            if (n >= len) {
                return -1;
            }
            var k = n >= 0 ? n : Math.max(len - Math.abs(n), 0);
            for (; k < len; k++) {
                if (k in t && t[k] === searchElement) {
                    return k;
                }
            }
            return -1;
        }
    }
})(this, this.jQuery);
share|improve this question
1  
SM? Really? –  Florian Margaine Nov 20 '12 at 15:54
    
related –  rlemon Nov 20 '12 at 15:56
    
@FlorianMargaine Hahahaha, oh dear!! Didnt think of that!! It stands for Sine Macula. I may change that now! –  Ben Carey Nov 20 '12 at 15:56
    
@BenCarey And btw fix your indexOf shim. Right now, it shims all the time even when the method exists. See the official shim. –  Florian Margaine Nov 20 '12 at 16:09
    
@FlorianMargaine Thank you very much, have amended my script :-) –  Ben Carey Nov 20 '12 at 16:15

1 Answer 1

From a once over:

  • I like use strict in an IIFE
  • However, the second use strict in indexOf is overkill
  • You are not using the variable oldSM
  • Lots of comments are good as well
  • The below code is prepending really, and the hostname should be called out in separate variable or even better be derived from window.location

    // Complete the url by appending the query
    url = '//libraries.sinemaculammviii.com/'+query;     
    
  • From a design perspective, I am not sure what you are providing with your script, except for a shell over jQuery. jQuery even has jQuery.getScript() which could replace

    // Append the script tag to the end of the document
    script = document.createElement('script');
    script.type = 'text/javascript';
    script.src = url;
    $('head')[0].appendChild(script);      
    

    it also has jQuery.inArray which you could use instead of your indexOf shim.

share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.