I'm hoping this is a simple question. Which is the accepted best practice for callbacks?

Option 1:

function get (id, callback) {
    var topic = find(id);
    var err = new Error('Sorry, ' + id + ' is not a valid id.');
    err.status = 404;
    return (topic) ? callback(null, topic) : callback(err);
}

Option 2:

function get (id, callback) {
    var topic = find(id);
    var err = new Error('Sorry, ' + id + ' is not a valid id.');
    err.status = 404;
    return (topic) ? callback(undefined, topic) : callback(err);
}

Side note, find() returns undefined, not null.

Thanks in advance.

I'm hoping this is a simple question. Which is the accepted best practice for callbacks?

Option 1:

function get (id, callback) {
    var topic = find(id);
    var err = new Error('Sorry, ' + id + ' is not a valid id.');
    err.status = 404;
    return (topic) ? callback(null, topic) : callback(err);
}

Option 2:

function get (id, callback) {
    var topic = find(id);
    var err = new Error('Sorry, ' + id + ' is not a valid id.');
    err.status = 404;
    return (topic) ? callback(undefined, topic) : callback(err);
}

Side note, find() returns undefined, not null.

Thanks in advance.

• javascript
• node.js

• Well, what do Node's various API calls do? You checked, right? – T.J. Crowder Commented Aug 19, 2015 at 15:21
• 4 In any case, constructing an error you never pass back (in the case where find works) is not a good idea. – T.J. Crowder Commented Aug 19, 2015 at 15:23

3 Answers 3

I would do what the Node built-in API functions do.

A trivial experiment tells me that:

• open passes null for err on success.

• open passes null for the data parameter on failure.

A couple of other points:

1. Your code is always constructing an Error object, even if things worked. I wouldn't do that, it's pointless.

2. Your code is returning the result of calling the callback. That's unusual.

3. Your code is calling the callback synchronously as far as I can tell. (E.g., when calling get, the callback will occur before get returns.) Usually an async call is, you know, async. If you're doing things synchronously, like openSync and such, put Sync on the name and return the value directly rather than calling a callback.

Passing null as an argument deactivate default parameter value mechanism. If callback function has default parameter values and you send null for empty parameter, callback function going to use null, not default value of parameter.

function multiply(a=1, b=1){
  return a*b;
}

console.log(multiply(undefined, 2));
console.log(multiply(null, 2));

The convention is null as seen here: https://docs.nodejitsu./articles/errors/what-are-the-error-conventions.

As @T.J. Crowder says in the ment, don't construct the error if it won't be sent back.

function get (id, callback) {
    var topic = find(id);

    if (topic) {
        return callback(null, topic);
    }

    var err = new Error('Sorry, ' + id + ' is not a valid id.');
    err.status = 404;

    return callback(err);
}