Returning values in functions that don't need to return a value

Question

I'm trying out js2-mode, which is a kind of javascript IDE in emacs. One of the features is syntax error highlighting and style warnings. Since turning it on, I've noticed that I'm getting this one warning all over my code:

anonymous function does not always return a value

When I write a function whose responsibility is to call a callback, I often do not explicitly add a return value. Is that bad style?

For example:

function someFunc (requiredParam, callback) {
    if (!requiredParam) 
        return callback("missing required param")

    someAsyncMethod(requiredParam, function(someValue) {
        callback(null, someValue)
    })
}

where the return only serves to ensure the rest of the function doesn't get executed. Is that better written as

function someFunc (requiredParam, callback) {
    if (!requiredParam) 
        return callback("missing required param")

    return someAsyncMethod(requiredParam, function(someValue) {
        return callback(null, someValue)
    })
}

The latter style is the only way to get js2-mode to leave me alone. Or maybe it should be written as

function someFunc (requiredParam, callback) {
    if (!requiredParam) { 
        callback("missing required param")
    } else {
        someAsyncMethod(requiredParam, function(someValue) {
            callback(null, someValue)
        })
    }
}

That also gives me a js2 pass, but I have always found the former style to read better. Am I doing it wrong or is js2-mode too uptight?

Not returning a value from a callback is ONLY bad style if the caller of the callback is expecting a return value from it. If no return value is expected by the caller, then it is pointless to return something in the callback. In your code, you really ought to only use `return` statements when you're actually purposefully returning a value that will be used - otherwise it is very confusing to people reading your code what should be put in that return value. — jfriend00, Feb 02 '15 at 04:09
How about using `return` to stop the execution a function, to save on curly braces and an indentation level? — harumph, Feb 02 '15 at 15:12
A plain return statement by itself is fine. But `return callback();` or `return someAsyncMethod(...);` makes it look like you are purposely returning a value which I would not recommend if there is no return value from the function. In my opinion, code clarity of intent is more important than saving a line or two of bracing. — jfriend00, Feb 02 '15 at 15:30

score 0 · Accepted Answer · answered Feb 02 '15 at 22:42

The consensus at work, and following from jfriend00, is that it should be written like this:

function someFunc (requiredParam, callback) {
  if (!requiredParam) { 
    callback("missing required param")
    return
  }

  someAsyncMethod(requiredParam, function(someValue) {
    callback(null, someValue)
  })
}

js2-mode was right. My desire to write one-liners was making my code less clear.

Returning values in functions that don't need to return a value

1 Answers1