CODEWITHSUNDEEP
javascriptfunctioncallback
wilsonpage
wilsonpage

Reputation: 17610

Best way to setup optional callbacks in a function

What is the best practice for optional success and error callbacks in a function like this? Does this method make sense? It seems a little bloated to me.

Function Declaration:

var myFunc = function(myNumber, options){
    options = options || {};
    options.onSuccess = options.onSuccess || function(){};
    options.onError = options.onSuccess || function(){};

    var myNewNumber = myNumber * 2;

    if(newVar > 10){
        options.onSuccess(myNewNumber);
    }else{
        options.onError(myNewNumber);
    }
}

Calling it with callbacks:

myFunc(2,{
    onError: function(myNewNumber){
        // do stuff
    },
    onSuccess: function(myNewNumber){
        // do stuff
    }
})

Calling it without callbacks:

myFunc(2);

Upvotes: 1

Views: 1794

Answers (2)

Matt
Matt

Reputation: 75327

A few things to note;

  1. options.onSuccess = options.onSuccess || function(){}; is checking for the existance of the member, rather than checking its a function.

    You might want options.onSuccess = (typeof options.onSuccess == "function") ? options.onSuccess : function () { };

  2. The same goes for onError

  3. As as slight optimisation, you could point the empty function to the same function; rather than recreating it potentially twice. If you're using jQuery, jQuery defines jQuery.noop():

    options.onSuccess = options.onSuccess || jQuery.noop;
options.onError = options.onSuccess || jQuery.noop;

  4. In the situation where the callbacks are asynchronous, you would be leaving yourself open to the options.onSuccess and options.onError being changed after your check, but before the callback is fired;

    var myFunc = function(myNumber, options){
    options = options || {};
    options.onSuccess = options.onSuccess || function(){};
    options.onError = options.onSuccess || function(){};

    var myNewNumber = myNumber * 2;

    setTimeout(function () {
        if(newVar > 10){
            options.onSuccess(myNewNumber);
        }else{
            options.onError(myNewNumber);
        }
    }, 2000);
}

var obj = {
    success: function () { alert('foo'); },
    error: function () { alert('foo'); },
};

myFunc(10, obj);
delete obj.success;
delete obj.error;

    When the callback gets executed, success and error will be undefined.

Upvotes: 4

Richard Dalton
Richard Dalton

Reputation: 35803

I would do it by checking the existence of the functions before calling them:

var myFunc = function(myNumber, options){
    options = options || {};

    var myNewNumber = myNumber * 2;

    if(newVar > 10){
        if (options.onSuccess) { options.onSuccess(myNewNumber); }
    }else{
        if (options.onError) { options.onError(myNewNumber); }
    }
}

It depends how many times you are likely to call these callbacks. If it is all over the place then your way might be better, or at least cleaner code.

Upvotes: 2

Related Questions