If the execution of the bulk of a function is conditional on the input, which of these is a better way to implement?

Question

I'm wondering which of these is better and why. I often encounter situations in my daily work where I'm like "This algorithm works so long as the input isn't empty" or something like that. I usually just return early because, for some reason, the idea of wrapping almost the entirety of a function in an if conditions seem wrong to me. However, I know that some religions don't believe in early return statements.

Example:

(1)

function combine ( strings , separator )
{
    if ( strings.length > 0 )
    {
       retstring = strings[0];
       for ( var i = 1; i < strings.length; ++ i )
           retstring += (separator + strings[i]);
    }
    return retstring;
}

(2)

function combine ( strings , separator )
{
    if (strings.length === 0) return undefined;   
     retstrings =  strings[0];
     for ( var i = 1; i < strings.length; ++ i )
         retstring += (separator + strings[i]);   
     return retstring;
}

So which is better to go with in such situations?

@RickHitchcock That's true for the specific code the OP provided, but not for the question they are (I think) trying to ask. That issue can easily be solved by adding a `var`, but even if you do you're still back to the choice of two patterns. — machineghost, Nov 06 '15 at 23:38

score 1 · Answer 1 · edited May 23 '17 at 12:03

I'd say that neither is "better"; it's subjective.

And, unlike many subjective programming choices, this one isn't just a matter of personal preference. Rather, I think a good programmer will use both patterns, choosing which one based on what they want to express.

Pattern #1 says "if X do Y". Pattern #2 says "If !X, don't do anything else." Admittedly, those two are equivalent to any browser running your code.

But, to a human reading your code (eg. such as a co-worker who has to modify it) each pattern suggests different things about what is going on. Thus, my recommendation would be to try and determine which of the two patterns best describes what you are trying to communicate, and use that.

For instance, many functions have "if this isn't relevant logic", and that is best expressed with pattern #2:

function doStuffIfLoggedIn(user) {
    if (!user.isLoggedIn()) return;

    doStuff();
}

But it's also fairly common to do something if a particular option is provided, and that fits better with the first pattern:

function format(word, capitalize) {
    if (capitalize) {
        word = string.toUpperCase();
    }
    returns word;
}

If either is equally valid (and I find this happens fairly often) then it does come down to a matter of preference. Personally, in those "either is valid" cases I opt for #2; all else being equal it results in less indentation, which I (subjectively) find easier to read.

But really, the important thing (IMHO) is to think about how your code will look to the person who comes after (and that might even be you, a year later when you've forgotten why you wrote it that way). The browser doesn't care either way, and your co-workers will be able to understand either one, but using the one that best represents the situation can offer a critical clue about the code's function to whoever reads it later.

EDIT

To your point about:

some religions don't believe in early return statements

I think the idea there is that multiple return statements can make the code more complicated. When your function has lots of ways of exiting it can become hard to understand its logic, because to interpret a latter part, you have to reason through whether any of the earlier parts prevented you from getting there.

However, the Stack Overflow consensus is that, while it's a good idea to avoid excessive return statements, using a few properly can make your code more readable, and thus are a good idea.

See: Should a function have only one return statement?

score 0 · Answer 2 · answered Nov 06 '15 at 23:35

There is a built-in array method that does what your functions do: join()

function combine(strings, separator) {
  return strings.join(separator);
}

console.log(combine(['this','is','a','test'], '...')); //this...is...a...test

But if join() didn't exist, I'd recommend a variation on your first code. You don't have to explicitly return undefined. If you don't include a return statement, the function will automatically return undefined:

function combine(strings, separator) {
  if (strings.length) {
    var retstring = strings[0];
    for (var i = 1; i < strings.length; ++i)
      retstring += (separator + strings[i]);
    return retstring;
  }
}

console.log(combine(['this','is','a','test'], '...')); //this...is...a...test

console.log(combine([], '...'));  //undefined

If the execution of the bulk of a function is conditional on the input, which of these is a better way to implement?

2 Answers2