JavaScript Module Pattern with AJAX

I have an application which currently uses AJAX for CRUD operations on a simple Person object. The following script works fine, but I'm looking for some pointers on how to structure my code. Currently, I have four methods for CRUD operations, and I'll extend my person module as needed, but I was hoping to get an idea of whether or not this is a nice way to manage code.

```
$(document).ready(function () {
//open the modal
//the update, delete and insert functions come from boxes present in a modal not shown
$('#openModal').click(function () {
$('#contact').modal();
});
//insert
$('#btnSavePerson').click(function () {
//is it a good idea to have the data objects inside all of these functions?
var data = {
personId: $('#tbPersonId').val(),
firstName: $('#tbFirstName').val(),
lastName: $('#tbLastName').val()
};
person.insert(data);
});

//delete
$('#btnDeletePerson').click(function () {
var personId = $('#tbDelete').val();
person.remove(personId);
});

//update
$('#btnUpdatePerson').click(function () {
var data = {
personId: $('#tbPersonId').val(),
firstName: $('#tbFirstName').val(),
lastName: $('#tbLastName').val()
};
console.log(JSON.stringify(data));
person.update(data);
});
//get
$('#getPeople').click(function () {
person.get();
});
//*person module

var person = (function () {
//the ajax object will be passed and shared between
//the functions here (type and dataType dont change in th

Right, I'm on a break, so this is just a few pointers to help you along. I'll probably get back to this answer and add some more details in days to come...

the first thing I noticed is that you declared your person module somewhere half-way your ready callback function. Though the variable person will be hoisted to the top of the scope the IIFE (Immediatly invoked function expression) that assigns it cannot be hoisted. It's an expression, so your code evaluates to:

$(document).ready(function()
{
    var person;
    //some code, using person as a module
    person = (function()
    {
    }());//assignment happens here!
});

Though it's very unlikely this will cause problems, it's generally a good idea to move the assignment to the top of the scope:

$(document).ready(function()
{
    var person = (function()
    {
    }());
    //rest of your code here...
});

Another thing that sort of annoys me is your use of window:

window.setTimeout

But at the same time, you're using:

console.log();

without the window reference. That's inconsistent and, if anything, quite redundant. JS resolves the window name in the same way as it resolves console or setTimeout: it scans the current scope, if it can't find the name there, it moves up to a higher scope until it reaches the global namespace/scope.

As you may know, the global NS is, effectively an object that has no name. (try console.log(this === window) in your console). This object has a property, called window, which is nothing more than a circular reference to this nameless object. Hence console.log(this.window === window); console.log(this.window === this) will both log true. As a result setTimeout will be very, very marginally faster than window.setTimeout anyway, so I'd suggest you loose the window where you can.

Next: the person module isn't exactly stand-alone, in it, you assume $ to be the jQuery object, and you expect it to be defined in a higher scope. I wouldn't do that. Here's why:

var dependency = {foo: function()
{
    return 'bar';
}};
var unsafeModule = (function()
{
    return { getBar : dependency.foo};
}());
unsafeModule.getBar();//returns bar, fine...
dependency = 213;
unsafeModule.getBar();//ERROR

A better, safer approach would be:

var dependency = {foo: function()
{
    return 'bar';
}};
var saferModule = (function(d)
{
    return { getBar : d.foo};
}(dependency));

By passing a reference to the object my module, that object remains in scope, and my module will continue to work just fine 'n dandy, no matter how many times the dependency variable gets reassigned.
But even better would be:

var dependency = {foo: function()
{
    return 'bar';
}};
var saferModule = (function(d)
{
    var mod = {setter: function(dep)
    {
        d = dep;
        this.getBar = d.foo;
        delete (this.setter);
        return this;
    }};

    if (d === undefined) return mod;
    return mod.setter(d);
}());//no dependency passed!
d.setter(dependency);//sets dependency
d.getBar();//works

When we apply this logic to your module, you can safely move the person module declaration to another file, or outside of the $(document)ready callback's scope:

var person = (function()
{
    var mod = { remove : function(){},
                get : function(){}};//declare your module
    return {init: function($)
    {
        var p;
        if($ instanceof jQuery)
        {
            delete(this.init);//unset init method
            for (p in mod)
            {//copy module properties into person
                this[p] = mod[p];
            }
            return this;
        }
        throw {message: 'Module needs jQ to work', code: 666};
    }};
}
$(document).ready(function()
{
    person.init($);//pass jQ ref to module, it'll init itself.
});

Lastly, a side-note. You're trying to implement the module pattern. That's good, but a module requires a closure. You can use closures all over the place to avoid, for example, excessive DOM queries.

For example:

$('#openModal').click(function () {
    $('#contact').modal();
});

Now each time I click on $('#openModal'), the callback function gets called. this function will query the DOM, because $('#contact') is a DOM selector. If I click this element 10 times, the DOM will be queried 10 times. A closure can reduce this:

$('#openModal').click((function(contact)
{
    return function()
    {
        contact.modal();//use ref in scope, no more DOM lookups required
    };
}($('#contact')));//pass DOM ref here, query dom once

These sort of, seemingly trivial, optimizations are easy, but can make a difference the bigger your script gets, and the more interactive your site has to be.

Ok, another update. I've noticed you write code like this:

$('#getPeople').click(function()
{
    person.get();
});

This is typical jQ code: passing an anonymous function to the event handler. When a callback funct

StackExchange Code Review Q#35316, answer score: 7