JavaScript Module Pattern with AJAX

Question

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 this example)
                var ajax = {
                    type: "POST",
                    url: "",
                    data: {},
                    dataType: "json",
                    contentType: "application/json",
                    success: {},
                    error: {}
                }

                //************ insert
                function insert(data) {
                    ajax.url = '../Service.asmx/InsertPerson';
                    ajax.success = function () {
                        console.log('success before setTimeout');
                        var successMessage = $('<div>').text('Successfully added to the database...')
                                                       .css('color', 'green')
                                                       .attr('id', 'success');
                        $('.modal-body').append(successMessage);
                        window.setTimeout(function () {
                            $('.modal-body').each(function () {
                                $(this).val('');
                            });
                            $('#contact').modal('hide');
                        }, 1000);
                    }
                    ajax.data = JSON.stringify(data);
                    ajax.error = function (xhr, ajaxOptions) {
                        console.log(xhr.status);
                        console.log(ajaxOptions);
                    };
                    $.ajax(ajax);
                }
                //************* delete
                function remove(data) {
                    ajax.url = '../Service.asmx/DeletePerson';
                    console.log('working string: ');
                    var obj = { personId: data };
                    console.log(JSON.stringify(obj));
                    ajax.data = JSON.stringify(obj);
                    console.log(ajax.data);
                    ajax.success = function () {
                        console.log('person successfully deleted');
                        var successMessage = $('<div>').text('Person successfully deleted from the database')
                                                       .css('color', 'red');
                        $('.modal-body').append(successMessage);
                        window.setTimeout(function () {
                            $('.modal-body input').each(function () {
                                $(this).val('');
                            });
                            $('#contact').modal('hide');
                        }, 1000);
                    };
                    ajax.error = function (xhr, ajaxOptions) {
                        console.log(xhr.status);
                        console.log(ajaxOptions);
                    }
                    $.ajax(ajax);
                }
                //*************** update
                function update(data) {
                    ajax.url = '../Service.asmx/UpdatePerson',
                    ajax.data = JSON.stringify(data),
                    ajax.success = function () {
                        console.log('update was successful');
                        var successMessage = $('<div>').text('Record successfully updated...')
                                                       .css('color', 'green');
                        $('modal-body').append(successMessage);
                        window.setTimeout(function () {
                            $('.modal-body input').each(function () {
                                $(this).val('');
                            });
                            $('#contact').modal('hide');
                        }, 1000);
                    };
                    ajax.error = function (xhr, ajaOptions) {
                        console.log(xhr.status);
                    };
                    $.ajax(ajax);
                };
                //************** get
                function get() {
                    //****** appropriate to have this function here?
                    function style(json) {
                        json = $.parseJSON(json);
                        var $personArray = [];
                        for (var obj in json) {
                            var $person = $('<div>').addClass('person');
                            for (var prop in json[obj]) {
                                var label = $('<span>').text(prop).addClass('badge pull-left').appendTo($person);
                                var propertyData = $('<span>').text(json[obj][prop]).addClass('pull-right').appendTo($person);
                            }
                            $personArray.push($person);
                        }
                        return $personArray;
                    }
                    ajax.url = '../Service.asmx/GetPersons';
                    ajax.success = function (data) {
                        data = data.d;
                        console.log('ajax successful');
                        $('body').append(style(data));
                    };
                    ajax.error = function (xhr, ajaxOptions) {
                        console.log(xhr.status);
                        console.log(ajaxOptions);
                    }
                    $.ajax(ajax);
                }
                //*********** public methods
                return {
                    insert: insert,
                    remove: remove,
                    update: update,
                    get: get
                };

            })();

        });

HTML in case someone wants to see it:

<body>
    <div class="navbar navbar-inverse navbar-static-top">
        <div class="container">
            <a href="#" class="navbar-brand">ui</a>
            <button class="navbar-toggle" data-toggle="collapse" data-target=".navHeaderCollapse">
                <span class="icon-bar"></span><span class="icon-bar"></span><span class="icon-bar">
                </span>
            </button>
            <div class="collapse navbar-collapse navHeaderCollapse">
                <ul class="nav navbar-nav navbar-right">
                    <li class="active"><a href="#">Home</a></li>
                    <li><a href="#">Blog</a></li>

                    <li><a id="openModal"  href="#contact">Contact</a></li>
                </ul>
            </div>
        </div>
    </div>

   <p id="getPeople" class="btn btn-block">get them peoples</p>
    <div class="modal fade" id="contact" role="diaglog">
        <div class="modal-dialog">
            <div class="modal-content">
                <div class="modal-header">
                <h2>contact: </h2>    

               </div>
               <div class="modal-body">
               The improtant information from the database is going to be displayed here. 
               <span class="label label-success">Person ID: </span><input id="tbPersonId" class="warning" /><br />
               <span class="label label-success">First Name:</span><input id="tbFirstName" /><br />
               <span class="label label-success">Last Name: </span><input id="tbLastName" /><br />
               <span class="label label-danger">Delete box: </span><input id="tbDelete" />
               </div>
               <div class="modal-footer">
                <a class="btn btn-default" id="modalClose" >Close</a>
                <a class="btn btn-primary" id="btnSavePerson">Save changes</a>
                <a class="btn btn-danger"  id="btnDeletePerson">Delete person</a>
                <a class="btn btn-block"   id="btnUpdatePerson">Update Person</a>

               </div>
               </div>

        </div>
    </div>
</body>

Answer 1

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 function (ie the anonymous function) merely calls another function, wouldn't it be easier, shorter and more efficient to just pass a reference to the function you need in the first place?

$('#getPeople').click(person.get);

It'll do pretty much the same thing, is shorter to write and saves on a pointless function object.
The only downside being: you're losing the call-context (within the person.get function this won't point to person anymore). So I had a look at your person.get function. I'll paste it here, with some comments. I'll address some issues further down

function get()
{
    //****** appropriate to have this function here?
    // NO, this function will be redeclared each time person.get is called
    //it should be defined somewhere else, in a higher scope
    function style(json)
    {
        json = $.parseJSON(json);
        var $personArray = [];
        for (var obj in json)
        {//still best to filter a for-in loop
            var $person = $('<div>').addClass('person');//again, DOM query?
            for (var prop in json[obj])
            {//I can't see where you're using the label var...
                var label = $('<span>').text(prop).addClass('badge pull-left').appendTo($person);
                var propertyData = $('<span>').text(json[obj][prop]).addClass('pull-right').appendTo($person);
            }
            $personArray.push($person);
        }
        return $personArray;
    }
    ajax.url = '../Service.asmx/GetPersons';
    ajax.success = function (data)
    {//this callback can be reused, too
        data = data.d;
        console.log('ajax successful');
        $('body').append(style(data));//avoid DOM access!
    };
    ajax.error = function (xhr, ajaxOptions)
    {//another callback to be avoided!
        console.log(xhr.status);
        console.log(ajaxOptions);
    }
    $.ajax(ajax);
}

So, let's consider what the get function actually needs to do its job:

• a style function
• a couple of callbacks (success and onerror) for the ajax object literal, and its own URL
• A couple of DOM elements, for example the $('body'). That's a pretty static element, I do believe.

So instead of getting all these objects, why not define/declare them when we're creating our get function, and reuse them every time this function is called? Instead of declaring function get inside the IIFE that returns the person module, we could write something like:

var get = (function(body)
{
    var successCallback, style,
        errorCallback = function(xhr, ajaxOptions)
    {//or declare as local var to function
        console.log(xhr.status);
        console.log(ajaxOptions);
    };
    successCallback = function (data)
    {
        console.log('ajax successful');
        //use DOM reference, parse it here already... SRP!
        body.append(style($.parseJSON(data.d)));
    };
    style = function(json)
    {
        var personArray = [], obj, prop, person;//declare at top of scope
        for (obj in json)
        {//I do hope this isn't an array!
            if (json.hasOwnProperty(obj))
            {//best to check, still
                person = $('<div class="person"'>);//avoid addClass call
                for (prop in json[obj])
                {
                    if (json[obj].hasOwnProperty(prop))
                    {//no point in assigning return val to vars you do not use
                        $('<span class="badge pull-left">')
                                 .text(prop).appendTo(person);
                        $('<span class="pull-right">')
                                 .text(json[obj][prop]).appendTo(person);
                    }
                }
                personArray.push(person);
            }
        }
        return personArray;
    };
    //the actual get function:
    return function()
    {
        ajax.url = '../Service.asmx/GetPersons';
        ajax.success = successCallback;
        ajax.error = errorCallback;
        return $.ajax(ajax);
    };
}($('body')));//either pass as argument

Now what I've done here is sort-of created a get module within the main person module. You can (and probably eventually will) work some more on this, since all of your person module functions perform ajax requests, you might add a more generic getAjax function that you choose not to expose.
A function that gouverns the ajax object, and keeps track of the callbacks and urls each function in the module requires, and that sets the ajax object accordingly:

var doAjax = function()
{
    var ajax = {},//your ajax object literal
    callers = {get: {url: '../Service.asmx/GetPersons',
                     success: function(){},
                     error: function(){}},
               insert: {url: '../Service.asmx/InsertPerson',
                        success: function(){}},
               default: {url: 'some/default',
                         success: function(){}}
    };
    return function(data,caller)
    {
        var settings,p;
        caller = caller || 'default';
        settings = callers[caller] || callers.default;
        for (p in settings)
        {
            if (settings.hasOwnProperty(p)) ajax[p] = settings[p];
        }
        return $.ajax(ajax);
    };
}());

Now all your module functions can just call this function like so:

doAjax(myData, 'insert');
//or
doAjax(undefined, 'get');

Oh, and this code would go here:

var person = (function()
{
    var doAjax = (function(){}());//in module!
}());