7

Which function callback structure is the best to use in Javascript and why? I have seen these two options use quite a lot. Are there any more?


Option A:

// DECLARATION
function funcA(required, success, error, options){

  // if there is an error in your function return 
  // and run error function
  if(errorHappens){ return error('an error') };

  // if no error happened: run the success function
  success('all good here is your var back', required);
}

.

// USAGE
funcA('this is required', function(result){

  // all good, do something with the result :)

},
function(error){

  // tell the user about the error
  alert('error happened')  
},{
  optionalThing: ':-)'
});


Option B:

// DECLARATION
function funcB(required, callback, options){

  // if an error happens run the single callback with
  // the err param populated and return
  if(errorHappens){ return callback('an error') };

  // if no error happened: run the callback with 'null'
  // as the error param.
  callback(null, 'this is a result');
}

.

// USAGE
funcB('this is required', function(err, result){
  if(err){ return alert('error happened') };

  // all good do something with the result :)

},{
  optionalThing: ':-)'
}
wilsonpage
  • 17,341
  • 23
  • 103
  • 147

4 Answers4

7

It depends on the environment, for node.js

I would personally recommend you stick to callback as the last parameter

function doSomething(data, options, cb) {
  ...
  if (err) return cb(err);
  cb(null, result);
}

doSomething(data, { ... }, function (err, data) {
    if (err) throw err;
    ...
});

Mainly because that's the coding style used in the node community. If you're not interacting with node.js at all then you should probably use the style most common to the environment you're interacting with.

If your environment mainly revolves around jQuery then I would just return a $.Deferred object from your doSomething function

function doSomething(data, options) {
  return $.Deferred();
}

$.when(
  doSomething(data, { ... })
).then(
  function () { /* success */ },
  function () { /* error */ }
);

Edit:

function doSomething(data, options, cb) {
  if (typeof options === "function") {
    cb = options;
    options = null;
  }
  ...
}
UpTheCreek
  • 31,444
  • 34
  • 152
  • 221
Raynos
  • 166,823
  • 56
  • 351
  • 396
  • Does that mean you have to have an empty hash in the middle for a call with no options? – wilsonpage Nov 23 '11 at 15:13
  • @pagewil no, it's an optional parameter. Use your standard optional parameter logic. Alternatively you let the first parameter be a hash and the second be a callback, it all depends on what your API is. – Raynos Nov 23 '11 at 15:16
  • Whithout it why doesn't the `cb()` take the place of the `options` hash as the second parameter? – wilsonpage Nov 23 '11 at 15:18
  • @pagewil it does. So you can call it as `doSomething(data, cb)` or `doSomething(data, hash, cb)`. I recommend the callback last style because everyone in the node community does it. – Raynos Nov 23 '11 at 15:20
  • what code should you use inside the function to account for this optional parameter order? – wilsonpage Nov 23 '11 at 15:22
  • @pagewil see edit, it's a standard technique. – Raynos Nov 23 '11 at 15:25
  • Always wanted to know the best way of doing that!! Thanks :) – wilsonpage Nov 23 '11 at 15:34
3

It doesn't really matter what you pick. I'd prefer B because I can add more error codes without changing the function definition.

You can also use the another pattern where your 'callback' is a hash of functions {error: function(){}, success: function(){}}. And this is extensible and quite common in JS libraries.

Candide
  • 30,469
  • 8
  • 53
  • 60
3

I prefer passing an object literal in the form :

var callback = {
    success : function(){},
    error : function(){},
    complete : function(){}
};

and use it in this manner :

function(callback){
    try{
        /*...processing...*/
        if(success)
            typeof callback.success === 'function' && callback.success(result);
        else 
            throw new Error('unsuccesfull');
    }catch(err){
        typeof callback.error === 'function' && callback.error(err.message);
    }finally{
        typeof callback.complete === 'function' && callback.complete() || typeof callback === 'function' && callback();
    }
}

The main advantage is that in this way, I can provide a callback for every result status, but also I can omit any status callback I want. I can even pass just a function (instead of an object) which will be the equivalent of the complete callback.

gion_13
  • 41,171
  • 10
  • 96
  • 108
  • That's ugly. That clearly screams that your object should be an event emitter or an event target so you can just `this.emit("success", result)` – Raynos Nov 23 '11 at 15:27
1

Neither.

I prefer a full-featured version with a hash of arguments, and shortcut methods with "common" arguments as parameters.

Which makes the most sense depends on usage patterns of the function in question.

Dave Newton
  • 158,873
  • 26
  • 254
  • 302