HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavascriptMajor

Possible antipattern: looping until a search condition is met

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
loopingantipatternmetconditionsearchuntilpossible

Problem

Whenever I write a loop, I always use closure if it's appropriate by way of a self-executing anonymous function. I will typically write my searching loops as follows:

var i = 0, length = myArr.length,
    fe = null;

for (; i < length; i++) {
    if ((function (el) {
            if (el.searchCondition) {
                fe = el;
                return true;
            }
        }(myArr[i])) === true) {
        break;  
    }
}

return fe;


I thought it was minimal code and pretty clean. I know it's not the easiest to read - but is there any reason why I should not be doing it this way?

Solution

I know it's not the easiest to read - but is there any reason why I should not be doing it this way?

You answered your own question already. It's not easy to read, which is a very good reason not to do it this way.

In addition to the readability, your code will (probably, I didn't profile it) also perform worse than the simple solution (because of the extra function call).

So you have two points against this pattern. What are the advantages?

Two other points: if (exp === true) adds extra complexity. I would just write if (exp), and let the function return false. And you should assign i right were you need it, inside the loop.

If you consider all those points, your code might look like this:

var length = myArr.length,
    fe = null;

for (var i = 0; i < length; i++) {
    if (myArr[i].searchCondition) {
        fe = myArr[i];
        break;  
    }
}

return fe;


If you don't like that the array is accessed twice, save the value in a temporary variable. Or just return directly like @dusky did:

var length = myArr.length;

for (var i = 0; i < length; i++) {
    if (myArr[i].searchCondition) {
        return myArr[i]; 
    }
}

Code Snippets

var length = myArr.length,
    fe = null;

for (var i = 0; i < length; i++) {
    if (myArr[i].searchCondition) {
        fe = myArr[i];
        break;  
    }
}

return fe;
var length = myArr.length;

for (var i = 0; i < length; i++) {
    if (myArr[i].searchCondition) {
        return myArr[i]; 
    }
}

Context

StackExchange Code Review Q#66839, answer score: 28

Revisions (0)

No revisions yet.