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'd want an advise about the following idea that I have for organizing JavaScript when it comes to creating an HTML page. My group will be starting a new project and the approach that I'm trying to come up with will be large scale because every HTML page will be using it. Below I'll show a sample code and I'd like it to be reviewed and advised if you think that's a good approach. I did not find anywhere anything better it seems.

There are 4 files:

  • ${root}/billing.html: contains an input box for the name on credit card
  • ${root}/js/com/mycompany/common/common.js: initializes logging and error handling
  • ${root}/js/com/mycompany/common/Url.js: class that is used to perform an AJAX call
  • ${root}/js/com/mycompany/aproject/billing.js: initializes things on the billing page

common.js:

var com_mycompany_common_common = function() {

    function log(message) {
        console.log((new Date()) + ': ' + message);
    }

    function init() {
        window.onerror = function(message) {
            log('Unhandled error: ' + message);
        }
    }

    return {
        log: log,
        init: init
    }
} ();

$(document).ready(function() {
    try {
        com_mycompany_common_common.init();
    } catch (e) {
        console.log('Error during initialization: ' + e);
    }
});

Url.js:

function com_mycompany_common_Url(url) {    
    this.url = url;
}

com_mycompany_common_Url.prototype.addParameter(name, value) {
    this.url += '?' + name + '=' + value;
}

com_mycompany_common_Url.prototype.ajax() {
    com_mycompany_common_common.log('Send ajax to: ' + this.url);
}

billing.js:

var com_mycompany_aproject_billing = function() {

    function init() {
        $('#submitButton').click(function() {
            Url url = new com_mycompany_common_Url('http://bla.com/process/billing');
            var creditCardName = $('#ccName').val();
            url.addParameter('name', creditCardName);
            url.ajax();
        }
    }

    return {init: init};
} ();

$(document).ready(function() {
      try {
           com_mycompany_aproject_billing.init();
      } catch (e) {
           com_mycompany_common_common.log('Error during initialization: ' + e);
      }
});

billing.html:

<!DOCTYPE html>

<html>
    <head>
        <title>Billing</title>
    </head>
    <body>
        Enter name on credit card: <input type="text" id="ccName" /><br><br>
        <button id="submitButton">Submit Payment</button>

        <script type="text/javascript" src="http://code.jquery.com/jquery-latest.js"></script>
        <script type="text/javascript" src="js/com/mycompany/common/common.js"></script>
        <script type="text/javascript" src="js/com/mycompany/common/Url.js"></script>
        <script type="text/javascript" src="js/com/mycompany/aproject/billing.js"></script>
    </body>
</html>
share|improve this question
    
@Raynos This is the stupidest comment I've ever seen. Can you please explain why your eyes are bleeding? –  SBel Jan 28 '12 at 12:46
    
@Raynos I did not test it but my question is beyond running this code, I want to talk to people if this is a good strategy to apply to a large application. Things like code reuse and easy unit testing, with many people involved. You may be right in that maybe this forum is not meant for architectural questions but I don't know any other appropriate one. –  SBel Jan 28 '12 at 13:31
    
@Raynos I'll try not to from now on. –  SBel Jan 28 '12 at 13:40
    
I'll try not be such as ass from now on ;) –  Raynos Jan 28 '12 at 13:42

2 Answers 2

You have globals everywhere. Your cross module communication strategy is "uniquely named globals everywhere". I assume you know globals are bad but modul8 has a good article about it

Use a module bundling strategy using tools like browserify or modul8

// common.js
function log(message) {
    console.log((new Date()) + ': ' + message);
}

window.onerror = function(message) {
    log('Unhandled error: ' + message);
}

module.exports = { log: log };

You just got rid of those ugly company names as your namespacing strategy. And you also removed that ugly boilerplate code. Here your common.js file exports an object with the log method

// Url.js
var log = require("common.js").log;

function Url(url) {    
    this.url = url;
}

Url.prototype.addParameter = function(name, value) {
    this.url += '?' + name + '=' + value;
};

Url.prototype.ajax = function() {
    log('Send ajax to: ' + this.url);
};

module.exports = { Url: Url };

You Url module explicitly requires your common file. It now has the log method.

//billing.js
var Url = require("Url.js").Url,
    $ = require("jQuery");

$('#submitButton').click(function() {
    var url = new Url('http://bla.com/process/billing');
    var creditCardName = $('#ccName').val();
    url.addParameter('name', creditCardName);
    url.ajax();
}

And billing explicitly requires Url and jQuery.

// billing.html
<!DOCTYPE html>

<html>
    <head>
        <title>Billing</title>
    </head>
    <body>
        Enter name on credit card: <input type="text" id="ccName" /><br><br>
        <button id="submitButton">Submit Payment</button>

        <script type="text/javascript" src="js/billing-bundle.js"></script>
    </body>
</html>

And your billing html file loads a single precompiled billing bundle.

share|improve this answer
    
This looks awesome. Let me read up on it and get back. –  SBel Jan 28 '12 at 13:39
    
I'm trying to grasp how browserify works behind the scenes. I want to ask you if I understand it correctly. There is a back-end service running. You make a single request and it returns JavaScript that is probably minified and all that stuff? –  SBel Jan 28 '12 at 13:57
    
@SBel the idea is that these tools are build tools / compilers. You run them on your entry point file (billing.js) and they recursively find all dependencies and builds up a file with all the files in the right order so that require just works. This gist shows how require works and all your files are wrapped in define(function () { });. Of course you can do this on the fly but that's just slow, build it statically (just like you can gzip html/css on the fly but that's slow). –  Raynos Jan 28 '12 at 14:11
    
Aahhh, so you need to compile them yourself. If there doesn't exist some Eclipse plugin that auto compiles for you when you change a file then that will suck. But thanks for the general, I really appreciate it. And yeah, regarding your comment about on the fly would be slow I would expect a smart back-end service that would only "compile" if something changed, otherwise would cache. –  SBel Jan 28 '12 at 14:15
    
@SBel the on the fly compilation is useful for dev builds. Personally I use a makefile, so make runs the build step and opens the unit tests. Not ideal but pretty smooth. –  Raynos Jan 28 '12 at 14:35

How I normally do namespacing:

// File: MyNamespace.Log.js

if (typeof(MyNamespace) === "undefined") {
    MyNamespace = {};
}

MyNamespace.Logger = (function () {
    return {
        log: function (message) {
            console.log(message);
        },

        warn: function (message) {
            console.warn(message);
        },

        error: function (message) {
            console.error(message);
        }
    };
})();

// File: MyNamespace.Url.js
if (typeof(MyNamespace) === "undefined") {
    MyNamespace = {};
}

MyNamespace.Url = function (url) {    
    this.url = url;
};

MyNamespace.Url.prototype.addParameter(name, value) {
    this.url += "?" + name + "=" + value;
};

MyNamespace.Url.prototype.ajax() {
    MyNamespace.Logger.log("Send ajax to: " + this.url);
};

// File: MyNamespace.Billing.js
if (typeof(MyNamespace) === "undefined") {
    MyNamespace = {};
}

var MyNamespace.Billing = (function () {
    return {
        init: function () {
            $("#submitButton").click(function () {
                var url = new MyNamespace.Url("http://bla.com/process/billing");
                var creditCardName = $("#ccName").val();
                url.addParameter("name", creditCardName);
                url.ajax();
            });
        }
    };
})();

$(document).ready(function() {
    try {
        MyNamespace.Billing.init();
    } catch (e) {
        MyNamespace.Logger.log("Error during initialization: " + e);
    }
});

Note that it doesn't matter what order these files are included in, because there's only runtime dependencies. If you have definition-time dependencies, then they must be defined in the correct order. At this point, you could include them individually, or use something like YUI Compressor to bundle them.

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.