Make _.each break when false is returned

[melnikov-s]

Now that _.each no longer uses the native forEach it can break when false is returned.

[braddunbar]

As far as the change is concerned, I don't have a strong opinion either way. However, it will badly break backwards compatibility so we should wait until a major version change regardless.

[jdalton]

Out of the changes I've made in my own implementation, I've noticed some devs stumble over exiting early in _.each when they use things like CoffeeScript. That said, I still dig it, but ya definitely a back compat concern so may be appropriate to plan around a major bump if it's going to happen.

[jridgewell]

What about exposing breaker externally?

[jdalton]

What about exposing breaker externally?

In the past similar issues have been closed with recommendations to use _.some or _.every instead to exit out of the loop. That said, stylistically being able to use it with _.each fits better for some scenarios; it also helps that it's a popular convention in jQuery. For me, I didn't sweat it too much and lumped it in the same bucket as _.each supporting chaining (which Underscore does now).

[fritx]

It seems Array.prototype.forEach doesn't do so

[akre54]

For the historically inclined, Underscore used to expose the breaker as _.breakLoop() (and bafflingly the iterator was wrapped in a try catch). Now that we no longer use ES5's forEach, we could expose a sentinel _.breaker object, but this too may be confusing (see #596 and others).

forEach has no use for return values. So while returning false may not break any existing code I'd argue that it's surprising behavior that would better be handled by _.find or _.any, as the docs suggest.

[joshuacc]

I'm in favor of exposing a generic sentinel value, perhaps with a more generic name like _.DONE. It could then be used for other purposes, like signalling the end of a trampolined iterator.

[jdalton]

@akre54

forEach has no use for return values. So while returning false may not break any existing code I'd argue that it's surprising behavior that would better be handled by _.find or _.any, as the docs suggest.

As I mentioned above returning false may cause some edge gotchas with CoffeeScript because of its implicit returns, however I think that having _.each support exiting early expresses intent better than alternatives such as _.find or _.some.

@joshuacc

I'm in favor of exposing a generic sentinel value, perhaps with a more generic name like _.DONE.

That seems unnecessarily complex. jQuery cleared the way for returning false making it a common extension to each already. The way I see it is if it's in jQuery it's practically de facto anyways.

[joshuacc]

@jdalton Well, part of my desire for a non-false sentinel comes from experience using a short-circuiting each in CoffeeScript (which is admittedly not the normal experience). Due to CoffeeScript's implicit returns, if the last expression in the callback evaluated to false it was necessary to add an explicit return to prevent an unexpected break out the each. Probably not a huge deal, but I think there is a fair bit of overlap between CoffeeScript and Underscore usage.

The other part is that I can see a standard sentinel value being useful for contrib and other mixin suites. Though obviously contrib can create it's own sentinel if Underscore decides not to. Some previous discussion: documentcloud/underscore-contrib#20

[akre54]

As I mentioned above returning false may cause complications with CoffeeScript

Absolutely. Although in my experience devs tend to prefer CoffeeScript's iterator sugar to the Underscore helper methods so that may not be as big of a dealbreaker.

I think that having _.each support exiting early is expresses intent better than alternatives such as _.find or _.some.

Also agreed to an extent. Personally I find a normal break within a for loop to be the clearest of them all, and I think returning false is ugly, but I get what you're saying here. What's an example of a time _.some, _.find or _.any would fail to capture the meaning of a particular block of code?

[jdalton]

What's an example of a time _.some, _.find or _.any would fail to capture the meaning of a particular block of code?

With _.some (_.any) you're looking to see whether any value satisfies the predicate and with _.find you're wanting to find the value that satisfies the predicate. The goals of those methods don't really align with wanting to break out of a loop, which mentally maps well to _.each as it's often seen as a sugared for-loop. Also _.some and _.find need a truthy value which is at odds with wanting to cancel iteration (as jQuery picks up on, the intuition is to return false to cancel). Finally _.some, _.every, and _.find won't work well when chaining as they'll return a boolean or value respectively instead of the collection which you may want to process further.

[Nasicus]

Is the status still the same in here?

As far as I could see in 4fb42e4 the "breaker" object - and therefore the only way to break the each loop - has been removed. I guess this means that each will "never" support break? I find this kind of sad, since for me it's a quite important feature of a loop.

[jridgewell]

As far as I could see in 4fb42e4 the "breaker" object - and therefore the only way to break the each loop has been removed.

Outside devs were never able to access that object, only internal underscore functions.

Is the status still the same in here?

Yup.

[Nasicus]

Outside devs were never able to access that object, only internal underscore functions.

Ah didn't realize that.

However what speaks against to create (as already mentioned above), two sentinel constants like _.LOOP_BREAK and _.LOOP_CONTINUE (this variable is not really needed, but it may result in better readable/looking code) ?

One could then write his _each like this:

_.each([0,1,2,3], function(i){
    if (i === 0){
        return _.LOOP_CONTINUE; //or just use 'return;'
    }

    if (i === 2){
        return _.LOOP_BREAK;
    }   

    console.log(i);
});

Sure it's more "complicated" compared to the return falseof jQuery however it's also much more clearer if you read the code in my opinion.

[jgonggrijp]

I'm opposed to this. Maybe a special sentinel value, but not a common everyday value that lots of iteratees are likely to return by coincidence. Then still, break is a procedural thing. find is the real functional equivalent.