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 the following function which runs as expected and is defined in the window object:

function myFunction(url, timeout, type, sc, ec) {
    return $.ajax({
        url: url,
        timeout: timeout,
        type: type,
        success: function (data) {
            if (typeof sc === 'function') {
                sc(data);
            }
        },
        error: function (jqXhr) {
            if (typeof ec === 'function') {
                ec(jqXhr);
            }
        }
    });
}

Which works as expected.

However, I am now expanding this function to be a whole lot more complex. As a result to this, I was wondering whether I could get some style guides (I'm a bit OCD when it comes to things like this in JS).

Additional features I am adding are as follows:

  • showLoader - the ability to show a modal loader
  • loaderMessage - the message displayed if showloader === true
  • storeAs - how to store the downloaded data

The new function now looks as follows:

(function (window, document, $, undefined) {
    function myFunction(settings, sc, ec) {
        var options = $.extend({
            showLoader: false,
            loaderMessage: 'Loading...',
            storeAs: null,
            timeout: 60000,
            url: 'http://www.google.co.uk',
            type: 'GET',
            success: function (data) {
                if (options.storeAs) {
                    localStorage.setItem(options.storeAs, JSON.stringify(data));
                }

                if (typeof sc === 'function') {
                    sc(data);
                }
            },
            error: function (jqXhr) {
                if (typeof ec === 'function') {
                    ec(jqXhr);
                }
            }
        }, settings);

        if (options.showLoader) {
            showLoader(options.loaderMessage); // external function to show loader
        }

        return $.ajax(settings);
    }

    window.app = {
        comms: {
            ajax: function (options, sc, ec) {
                return myFunction(options, sc, ec);
            }
        }
    };
}(this, this.document, jQuery));

Anonymous function

I like to now define all my code within anonymous functions since it frees up the global name space.

However, I have two questions:

  1. Is it recommended/do I need to make use of the JavaScript new keyword? Is there any advantage to doing so?
  2. If so, then how would I go about using it? I have got to grips with the majority of the language, just not the new constructor/keyword?

Parameters

My second change will be to address the parameters - five parameters in JavaScript is quite readable. However, any more isn't.

I am thinking of using a settings object as seen above:

Couple of questions surrounding this however:

  1. Is it wise to use just one options object, as I am already doing here? $.ajax receives this settings object, but my settings object contains not just jQuery.ajax settings, but function settings - obviously jQuery will ignore these since it will use just the necessary options, but is there any reason to avoid this?
  2. I could combine the success callback, and error callback into the options object but this looks quite messy. Is it better to provide the callbacks as separate parameters or move those into the options callback too?

What is the difference between:

var options = $.extend({}, {
    showLoader: false,
    loaderMessage: 'Loading...',
    storeAs: null
}, settings);

And:

var options = $.extend({
    showLoader: false,
    loaderMessage: 'Loading...',
    storeAs: null
}, settings);

Is there anything else I should be doing which I am missing, which is generally recommended in JavaScript?

share|improve this question
    
Is it really named myFunction? –  200_success Jun 12 at 11:03
    
jquery, I assume? (If not, please fix the tags.) –  200_success Jun 12 at 11:05
    
No it's not - I just mocked up a quick example because my original code would be meaningless out of context. And yes correct - thank you. Chrome was being a bit buggy and not showing the recommendations for some reason. –  keldar Jun 12 at 11:09
    
Placeholder names such as myFunction would normally make a question hypothetical and therefore off-topic for Code Review. This is not the worst I've seen, but I'd still encourage you to post the question using realistic identifiers. –  200_success Jun 12 at 11:15
add comment

Your Answer

 
discard

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

Browse other questions tagged or ask your own question.