3

I recently realized that when I use a function to loop over an array and return a match, I actually don't need the return false/null at the end.

For example, if I have:

*Edited example. Original example was from a function I tried to simplify but forgot to change the name/context of. Sorry for the confusion. Here is a better example fitting the title of my question:

var hasKey = hasKeyMatch(key);
function hasKeyMatch(key) {
    for (var i = 0; i < array.length) {
        if (array[i].key === key) {
            return true;
        }
    }
};

I don't actually need a return false, as if there is no return of key, hasKey will be undefined. So I can still use hasKey as a boolean.

But is this considered good style? I realize having a 'back-up' return is a necessity in languages like Java, and so some people bring the habit to JS. But I think minimizing unnecessary returns would be ideal, though I have no idea of the actual cost of returning.

And I when I look at the answer to the below question, I am confused on why he chooses to return a variable that is already pushed to the desired array. I assume his return is intentional, and he isn't intended to store the returned variable anywhere. Are there benefits (like say garbage collection) of returning a variable at the end of a function?

drawImage using toDataURL of an html5 canvas

Community
  • 1
  • 1
user2736286
  • 563
  • 6
  • 18
  • 5
    This is a matter of opinion, but I for one am fine with an implied `undefined` return value for functions that return a matched value if found. Although the name of your function, `hasKeyMatch()`, implies that the return will be either `true` or `false`, whereas you return the same value that was passed to the function. So in this case it would make more sense (to me) if you returned the index in the array where the match was found, and `-1` if not found. (Which is what [`.indexOf()`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/indexOf) does.) – nnnnnn Sep 03 '13 at 21:52
  • I would have a function of that name return `true`or `false`. Anyway, don't return the key as a result - what if you seek a key that would evaluate to something falsy? The function would find it but return something to the effect of a denial. – npup Sep 03 '13 at 21:53
  • I prefer to return *boolean* values (true/false) vs. *truelean* values (truthy/not truthy) unless the latter makes sense (i.e. the values returned themselves are used for something besides their truthy/flasey nature. If the value returned *is* expected to be usable for other purposes (and not change truthy/falsey meaning), then return it. – user2246674 Sep 03 '13 at 21:56
  • 1
    Sorry, I added an unrefined example which caused off-topic discussion. I wasn't really looking for help with the actual example code, just advice on a conceptual/style level. – user2736286 Sep 03 '13 at 22:05
  • 1
    I don't think the discussion was off topic, because whether it makes sense to return `undefined` depends on what the function does. So my conceptual/style advice on the updated version of the function is that I'd prefer a `return false` at the end because the name `hasKeyMatch()` implies a boolean return. `undefined` makes sense to me only where the function will return the relevant value if found and `undefined` otherwise, not as a sort of pseudo-false. – nnnnnn Sep 03 '13 at 22:23
  • I would tend to be explicit about what gets returned. Although `undefined` is falsy, `undefined !== false`. The name of the method implies a true or false result, I would stick to that and return *only* true or false. If you were to rename the method to `getKeyMatch`, I would expect a different return type (ie- the key in case of a match, or undefined or null for no match). The point is that the method name should imply the return value. – Andrew Eisenberg Sep 03 '13 at 23:13

4 Answers4

1

I like to have method that are expected to return a boolean result (i.e., methods/functions that start with is or has) return true or false. JavaScript, of course, makes it easy to return undefined and it also assigns truthyness and falsiness to values that aren't true or false. I feel it is better to return true or false instead of the value of key. What use is returning key here anyway? You already have the value of key before you call the function.

It's hard to say why the author has chosen to return c and push it into the states array. Perhaps the states array is used for something else.

I realize having a 'back-up' return is a necessity in languages like Java, and so some people bring the habit to JS

I'm not entirely sure what you mean by this; I don't find myself doing this in Java.

Vivin Paliath
  • 94,126
  • 40
  • 223
  • 295
  • 1
    Regarding your last sentence, [here is an example](http://ideone.com/GHNJFi). You have to have an unconditional return in Java. – mpen Sep 03 '13 at 22:06
  • @Mark: not actually "unconditional" http://ideone.com/RvE4o3 – zerkms Sep 03 '13 at 22:10
  • @zerkms: Sorry I wasn't clear, your scenario is exactly what I meant by "unconditional" -- i.e., under all conditions, it will return something. I didn't mean that it couldn't be in an if/else statement. – mpen Sep 03 '13 at 22:13
  • @Mark: indeed, it's just about bad wording. It might have been something like "every branch of code should return a value of the declared type" – zerkms Sep 03 '13 at 22:14
  • @Mark Thanks for the explanation - that's more of an issue where every code-path must result in a return value if the method is expected to return something. – Vivin Paliath Sep 03 '13 at 22:18
1

(Ignoring the truthy/falsey debate)

I personally don't mind a function returning undefined where it makes sense, but I'd consider adding return void 0; to the end to show that it's intentional. A missing return statement at the end of a function suggests to me that it should never hit there. Performance isn't really an issue here.

mpen
  • 272,448
  • 266
  • 850
  • 1,236
1

Isn't it because you aren't specifying a return value function hasKeyMatch(key) in the method statements? Java and other languages generally require you to specify what you are actually returning. I say it would always be good practice to end the statement with a "default" return statement if things go haywire. JUST IN CASE. Some weird stuff happens and your code doesn't work at a specific spot it'll catch it and return something valid versus something that's undefined that could potentially break your code.

Edit:

It's always better to specify a undefined return call if that's what you are looking for also. If someone is reading this code they won't be able to tell what's actually suppose to be returned. That's the problem with scripting languages.

progrenhard
  • 2,333
  • 2
  • 14
  • 14
  • I think you missed something here... he knows it returns `undefined`, and that's documented, expected behaviour. Nothing went haywire. – mpen Sep 03 '13 at 22:12
  • @Marc It's always better to specify a undefined return call if that's what you are looking for also. If someone is reading this code they won't be able to tell what's actually suppose to be returned. That's the problem with scripting languages. – progrenhard Sep 03 '13 at 22:13
  • @progrenhard: Yes, I agree (hence my answer). The way you worded your last couple sentence suggests that the output was unexpected though. +1 for edit – mpen Sep 03 '13 at 22:16
  • _"That's the problem with scripting languages."_ - But it's well understood by JS programmers that functions return `undefined` by default. – nnnnnn Sep 03 '13 at 22:24
  • @Mark Yeah, totally i think the real question here is how much is the return statement actually effecting performance. Is it nothing? hardly anything? Is actually not stating a return statement worse because it has to somewhere completely different to return something? Or is return statement just simply costly? I believe its negligible. However, seeing the cost of a return statement would be interesting. – progrenhard Sep 03 '13 at 22:24
  • @nnnnnn It's a common practice sure. However, what if it was done unintentionally? Lots of confusion. This is based purely on clarity. I think it is a problem it is "well understood." – progrenhard Sep 03 '13 at 22:26
1

This is a matter of code solidity and you should always return false instead of undefined if the method name suggest that as a result.

As everything which has to do with code style it is not a matter of right or wrong (http://www.youtube.com/watch?v=zN-GGeNPQEg) but it is important if you intent others to understand what you are doing and avoiding stupid bugs. Solid code avoids the possibility of doing stupid things by behaving as expected.

For example: isA() && isB() || !isA() && !isB() can be written more shortly as isA() == isB(). If you had these two functions defined as one (isA) returning false and one (isB) returning undefined the second expression would not behave as one might suspect.

See: http://jsfiddle.net/wDKPd/1/

Scheintod
  • 7,953
  • 9
  • 42
  • 61
  • 1
    It seems the consensus is to take the 'defensive' approach and always explicitly return. Thanks everyone for the answers, I have no more uncertainty about this anymore :). Thank you Scheintod for providing a practical example of where relying on undefined just won't cut it. – user2736286 Sep 03 '13 at 22:41