views:

94

answers:

4

I'm refactoring a large javascript document that I picked up from an open source project. A number of functions use inconsistent return statements. Here's a simple example of what I mean:

var func = function(param) {
    if (!param) {
        return;
    }
    // do stuff
    return true;
}

Sometimes the functions return boolean, sometimes strings or other things. Usually they are inconsistently paired with a simple return; statement inside of a conditional.

The problem is that the code is complex. It is a parser that uses a multitude of unique RegEx matches, creates and destroys DOM nodes on the fly, etc. Preliminary testing shows that, in the above example, I could change the return; statement to become return false;, but I'm concerned that I may not realize that it had a negative impact (i.e. some feature stopped working) on the script until much later.

So my questions: Is there a benefit to using a blank return statement? Could this have been intentionally coded this way or was it just lazy? Can I change them all to return false;, or return null; or do I need to dig through every call and find out what they are doing with the results of those functions?

+4  A: 

"Blank return" statements can be used to transfer the control back to the calling function (or stop executing a function for some reason - ex: validations etc). In most cases I use blank return statement is when I'm doing some kind of a validation. However, I make it a point to set some indicator as to why the execution of the function is stopped. For example, set the "innerText" property on a DIV element with the error message.

In the code above, it looks like it is a validation. The function returns a "true" if everything went well. It looks like the calling function parses the return value, and if it is "true", next step of statements (in the calling function) are executed.

It is a good practice to return "false" instead of a blank return in the above example. That way you make it all uniform and make life easy for other programmers.

You could fix such inconsistencies; however, make sure you test all the changes thoroughly. It is a good practice to test each change you make to the code, however small it may be.

Josh
+3  A: 

Using return without a value will return the value undefined.

If the value is evaluated as a boolean, undefined will work as false, but if the value for example is compared to false, you will get a different behaviour:

var x; // x is undefined
alert(x); // shows "undefined"
alert(!x); // shows "true"
alert(x==false); // shows "false"

So, while the code should logically return true or false, not true or undefined, you can't just change return; to return false; without checking how the return value is used.

Guffa
I didn't consider undefined. Since you're the first to mention it, and your answer is similar to the others, I'm marking this as correct.
Stephen
+1  A: 

Changing your functions will actually alter the code because return; and return false; output different data types.

var test = function (x) {
    if (!x) {
        return;
    }
    else {
        return false;
    }
};

var a = test(true), b = test(false);

console.log(typeof b); // boolean
console.log(typeof a); // undefined  
Q_the_dreadlocked_ninja
Very good. Thank you!
Stephen
+1  A: 

What MIGHT be lost here (not direct with your example) is that you can then have a tri-state object:

var myfunc = function(testparam) {
    if (testparam == undefined) return;
    if (testparam) {
        return true;
    }
    else {
        return false;
    }
};

var thefirst = myfunc(true)
var thesecond = myfunc(false);
var thelast = myfunc();
alert("type:" + typeof thefirst+" value:"+thefirst);
alert("type:" + typeof thesecond+" value:"+thesecond);  
alert("type:" + typeof thelast+" value:"+thelast); 

these return:

> type:boolean:true 
> type:boolean:false
> type:undefined:undefined

note: null would return "undefined" in this example myfunc(null);

Mark Schultheiss
Thanks for the explanation.
Stephen