Breaking my javascript into modules, am I doing this right?

Question

I'm using the javascript module pattern, and have this so far:

var APP;
if(APP == undefined) {
  APP = {};
}

APP = (function() {
    var userId = -1;
    var privateVar = '';

    var _init = function($userId) {
       userId = $userId;
    };
    var _publicMethod = function($id){
        privateVar = id;
    };
    return {
        init = function($userId) {
             _init($userId);
         },
        publicMethod = function($id) {
           _publicMethod($id);
        }
    };
})();

I then have a common utils module:

APP.utils = (function(){

})();

And then per page I am planning on having a module, so I don't wireup events (button clicks etc) for no reason on pages where these DOM elements don't event exist:

APP.homePage = (function(){
     return {

     };
})();

So each module will have a init() method that I will call on the page to run things that have to be run (e.g. wiring up events, setting variables like say userId etc):

$(document).ready(function() {
   APP.init(<%= user.id %>);
   APP.homePage.init('abc');
});

So now if the files get too big, I can break them out into separate files also.

1. What if one module needs to call another, I guess the only way for this to work is to do this via the public api right? e.g. what if homePage needs userId, should I pass that in the homePage#init method?

2. How is my coding style, any obvious style that is not considered best practise?

3. Any comments on this approach? Is it good in general?

Answer 1

What if one module needs to call another, I guess the only way for this to work is to do this via the public api right?

Yes

e.g. what if homePage needs userId, should I pass that in the homePage#init method?

No. I'd not repeat the userId code in all modules, but offer a public getter for it in the default module.

Any comments on coding

This code

var APP;
if(APP == undefined) {
  APP = {};
}

APP = ...

is quite useless. You don't need to check for object existance here, because you overwrite it anyway. That also means that this code must be the first to execute. If you want to make the modules independent from load order, you'd need to use something like

var APP = (function(a) {
    var private_vars; // ...
    a.init = ...
    a.publicMethod = ... // add them to the object instead of creating new one
    a.getPrivate = function() {
        return private_vars;
    };
    return a;
})(APP || {}); // create one iff not already existing

// other file:
var APP = APP || {};
APP.utils = ... // add object to namespace

The code

var _publicMethod = function($id){
    privateVar = id;
};

looks a bit odd. First, the underscore usually denotes a semiprivate (public-but-not-to-be-used) attribute of objects and should not be used for variable names. That's not the case in here as the function will be exposed as the "publicmethod" property of APP. Use the underscore there if you want it. Second, there is no need to use a function expression here. The code is in the module's local scope, and using a function declaration both makes it available everywhere in that scope and allows naming the function. You should use

function publicMethod($id) {
    privateVar = id;
}
a.publicMethod = publicMethod;

Answer 2

The module pattern is, in my opinion, a really nice way to organize your code. To answer your questions:

1) Yes, your modules can only access methods and properties of other modules which have been exposed in the object they return.

2) I think your coding style looks pretty good. I'd make these changes:

APP = (function() {
    var _userId = -1;
    var _privateVar = '';

    var init = function($userId) {
       _userId = $userId;
    };
    var publicMethod = function($id){
        _privateVar = id;
    };
    return {
        init : init,
        publicMethod : _publicMethod
    };
})();

First, underscores are generally meant to denote "private" properties or methods. Secondly, you can do away with the extra functions in the object being returned and just point straight to the methods or properties you care about. This is generally referred to as the "Revealing Module Pattern", because even the public methods aren't defined within the returned object - they're simply referenced.

3) This approach is definitely a nice way to encapsulate code. You get the benefit of private and privileged methods, and you generally end up with a nicer API because you're only exposing things that need to be public.

Well done.