Skip to content
This repository has been archived by the owner on Apr 12, 2024. It is now read-only.

forEach should support break #263

Closed
IgorMinar opened this issue Feb 5, 2011 · 61 comments
Closed

forEach should support break #263

IgorMinar opened this issue Feb 5, 2011 · 61 comments

Comments

@IgorMinar
Copy link
Contributor

while continue support is already supported by accident, there is no way to do a break.

continue can be done by a simple return already:
//prints all elements but 2
forEach([1,2,3,4], function(i) {
if (i==2) return;
console.log(i);
}

break on the other hand isn't possible to achieve this way, we need to modify forEach to do that. There are three options;

  • throw a special exception which would be caught and matched by forEach
  • return a special value
  • making forEach stateful

I'm not a big fan of using exceptions to control execution flow in non-exceptional cases, so I'm for the second approach. Here is my proposal:
//prints all elements until 2 is reached == only "1"
forEach([1,2,3,4], function(i) {
//break() is a function that returns an immutable object,
//e.g. an empty string
if (i==2) return forEach.break();
console.log(i);
}

I also considered the third approach, but since you'll have to use return if you want to break from the middle of an iterator anyway, we might prevent issues with accidentally forgetting to return.

This third approach would from the user perspective look exactly the same as the code for the second approach above. From angular implementation point of view, things would be more complicated.

When this feature is implemented, we should look at our existing forEach instances and add break where needed.

We should also properly document both continue and break scenarios.

@vojtajina
Copy link
Contributor

+1 for this feature

I used just return true / false (break / continue) in couple of projects, not as verbosing as return forEach.break(), but shorter... (don't forget, outside angular, you have to type angular.forEach.break())

Or returning numbers, that would allow breaking nested loops as well:

// pseudo code
function forEach(items, callback, scope)
  for (var i in items)
    var retCode = callback.call(scope, i, items[i]);
    if (retCode) return retCode - 1;

// so you can break from nested loops:
forEach(multiarray, function(i, item) {
  return forEach(item, function(j, subItem) {
    if (a) return 0; // continue
    if (b) return 1; // break first
    if (c) return 2; // break from both loops
  });
});

Or might be more helpful to return key of the item that caused the break...
Just quick ideas, haven't thought about it properly...

@IgorMinar
Copy link
Contributor Author

we discussed this with Misko some time ago and realized that the implementation must be compatible with native forEach, which means that we'll have to use an exception (directly or indirectly) to abort the execution.

@vojtajina
Copy link
Contributor

Don't know - I don't like exceptions and especially when it comes to using exception for changing control flow...

@IgorMinar
Copy link
Contributor Author

It's not that I like it, but rather that it is the only option we have
when we want to be able to delegate to native forEach.

@JensRantil
Copy link
Contributor

+1 Needed this today.

I like your proposed solution, @IgorMinar.

@IgorMinar
Copy link
Contributor Author

Sorry.. I don't think that this will happen because if we start throwing exceptions to control execution flow we'll make debugging of apps a nightmare (break on exception will see false positives).

Maybe one day the native forEach will have an api to break. until then we can't do much about it.

I'm going to sadly close this issue since its obsolete. :-/

@ghost
Copy link

ghost commented Aug 30, 2012

Native each and every are just like forEach, except they can stop... or
am I missing something?

Herby

Igor Minar wrote:

Sorry.. I don't think that this will happen because if we start throwing
exceptions to control execution flow we'll make debugging of apps a
nightmare (break on exception will see false positives).

Maybe one day the native forEach will have an api to break. until then
we can't do much about it.

I'm going to sadly close this issue since its obsolete. :-/


Reply to this email directly or view it on GitHub
#263 (comment).

@ghost
Copy link

ghost commented Aug 30, 2012

Herby Vojčík wrote:

Native each and every are just like forEach, except they can stop... or
fix: some and every
am I missing something?

Herby

Igor Minar wrote:

Sorry.. I don't think that this will happen because if we start throwing
exceptions to control execution flow we'll make debugging of apps a
nightmare (break on exception will see false positives).

Maybe one day the native forEach will have an api to break. until then
we can't do much about it.

I'm going to sadly close this issue since its obsolete. :-/


Reply to this email directly or view it on GitHub
#263 (comment).

@ProLoser
Copy link
Contributor

ProLoser commented Sep 7, 2012

Why can't you return false similar to how $.each() does?

@jamie-pate
Copy link

continue and break cause exceptions which /could/ be caught (if you decided this was the road to go down)
IE:
> continue
"Can't have 'continue' outside of loop"
>> break;
"Can't have 'break' outside of loop"

FF:
>>> break

SyntaxError: unlabeled break must be inside loop or switch
[Break On This Error]   

>>> continue

SyntaxError: continue must be inside loop
[Break On This Error]   

Chrome:
break;
SyntaxError: Illegal break statement
continue;
SyntaxError: Illegal continue statement

instead of coding all these strings in, you could catch an illegal break and continue during init, then match that exception string in the forEach code

@mhevery
Copy link
Contributor

mhevery commented Sep 7, 2012

These are syntax errors, not runtime errors, and so they are of no help to
us.

On Fri, Sep 7, 2012 at 8:48 AM, jamie-pate notifications@github.com wrote:

continue and break cause exceptions which /could/ be caught (if you
decided this was the road to go down)
IE:

continue
"Can't have 'continue' outside of loop"

break;
"Can't have 'break' outside of loop"
FF:

break

SyntaxError: unlabeled break must be inside loop or switch
[Break On This Error]

break

with(_...reak }; (line 2)

continue

SyntaxError: continue must be inside loop
[Break On This Error]

break;
SyntaxError: Illegal break statement
continue;
SyntaxError: Illegal continue statement

instead of coding all these strings in, you could catch an illegal break
and continue during init, then match that exception string in the forEach
code


Reply to this email directly or view it on GitHubhttps://github.com//issues/263#issuecomment-8369315.

@IgorMinar
Copy link
Contributor Author

@Herby Array#forEach doesn't have any way of breaking or escaping the loop.

@ghost
Copy link

ghost commented Sep 14, 2012

@IgorMinar I said some or every which has.

@saurabhnanda
Copy link

Any possibility of having a new function called forEvery which gives the ability to break out of the loop without using exceptions? How bad would NOT being able to delegate to the native forEach be?

@PhantomRay
Copy link

This is a must have feature, otherwise we end up using for(..).

Or as saurabhnanda said, add another function.

@pocesar
Copy link
Contributor

pocesar commented Apr 17, 2013

damn, how is this not in, 2 years later?

@patrickkettner
Copy link

Perhaps angular could add an .every() method?

@cburgdorf
Copy link
Contributor

One thing I don't get from the thread is why delegating to the native forEach is so important. The feature could be implemented with a for loop I guess. What am I missing?

@saurabhnanda
Copy link

Efficiency? Are there any benchmarks around how much faster native forEach
is?

On Wed, Apr 17, 2013 at 12:01 PM, Christoph Burgdorf <
notifications@github.com> wrote:

One thing I don't get from the thread is why delegating to the native
forEach is so important. The feature could be implemented with a for loop I
guess. What am I missing?


Reply to this email directly or view it on GitHubhttps://github.com//issues/263#issuecomment-16489051
.

http://www.saurabhnanda.com

@patrickkettner
Copy link

@saurabhnanda definitely not the case - that is the whole purpose behind lodash's loops.

http://jsperf.com/angularjs-lodash-underscore-foreach/3

I would venture a guess that it is purely for consistency (jQuery's notwithstanding).

@saurabhnanda
Copy link

Wow! This is really interesting. Why is it that a simple for-loop in plain
ol' JS is faster than the native forEach? Also, why is lo-dash
significantly slower than the plain old JS for loop? Lo-dash seems to be
doing the same thing, except that its using a while loop --
https://github.com/bestiejs/lodash/blob/master/lodash.js#L2595

On Wed, Apr 17, 2013 at 12:07 PM, patrick kettner
notifications@github.comwrote:

@saurabhnanda https://github.com/saurabhnanda definitely not the case -
that is the whole purpose behind lodash's loops.

http://jsperf.com/angularjs-lodash-underscore-foreach/3

I would venture a guess that it is purely for consistency (jQuery's
notwithstanding).


Reply to this email directly or view it on GitHubhttps://github.com//issues/263#issuecomment-16489219
.

http://www.saurabhnanda.com

@patrickkettner
Copy link

@saurabhnanda
http://kitcambridge.be/blog/say-hello-to-lo-dash/
http://allyoucanleet.com/post/21624742336/jsconf-us-12-slides

mostly because the for loops have been optimized as much as possible, and the native methods are new, so they haven't been as heavily optimized.

as to why lodash is slower, i'm not sure, other than they are using a while loop rather than a for loop.

@cburgdorf
Copy link
Contributor

It would be nice to get an official statement from the core team again on why we would not implement that feature in the fashion lo-dash does.

/cc @IgorMinar @vojtajina @mhevery

@pocesar
Copy link
Contributor

pocesar commented Apr 17, 2013

@patrickkettner @saurabhnanda because you are calling an anonymous function everytime you iterate. less calls (aka plain loop) is obviously faster than thousands of functions calls in some ms', but of course, in some situations the "slower" is unnoticeable if you are iterating over less than 40 items for example (the jsperf is iterating over 1000 times)

@patrickkettner
Copy link

@pocesar ah-ha, thanks for the explanation. Sleepy minds do not valid test cases make.

@saurabhnanda looks like someone updated that test with a anonymous function version of the for loop
http://jsperf.com/angularjs-lodash-underscore-foreach/4

@jamie-pate
Copy link

That was me, final (final) test is here: (added every and fixed rawfor so it actually accesses the array)
http://jsperf.com/angularjs-lodash-underscore-foreach/7

native forEach does a lot more than the for loop, it has to check stuff!
The native forEach still goes twice as fast as the shim version that does the same thing
see http://es5.github.com/#x15.4.4.18

@cburgdorf
Copy link
Contributor

However, angular.forEach is slower than lodash and lodash does support break..

@ghost
Copy link

ghost commented Apr 18, 2013

forEach shouldn't support break, every/some does.
I said it long ago, others said it as well.
Angular should add some and every.

Christoph Burgdorf wrote:

However, angular.forEach is slower than lodash and lodash does support
break..


Reply to this email directly or view it on GitHub
#263 (comment).

@cburgdorf
Copy link
Contributor

So the only reason that wasn't implemented yet is to keep the same semantics as the native forEach? Fuck yeah, then call it every or some :)

gkalpak added a commit to gkalpak/angular.js that referenced this issue Oct 20, 2014
…Each()`)

NOTICE: This is just a POC implementation to see if there is interest for the feature.
        There are many performance improvements to be made and tests to be written (and run).
Sometimes it is useful to be able to iterate over an array/object, but abort the operation as soon
as some condition is met. This commit adds two methods that provide this functionality:
1. some:  Test whether some element in the array/object passes the test implemented by the provided
          function.
2. every: Test whether every element in the array/object passes the test implemented by the provided
          function.

Closes angular#263
gkalpak added a commit to gkalpak/angular.js that referenced this issue Oct 23, 2014
…Each()`)

NOTICE: This is just a POC implementation to see if there is interest for the feature.
        There are many performance improvements to be made and tests to be written (and run).
Sometimes it is useful to be able to iterate over an array/object, but abort the operation as soon
as some condition is met. This commit adds two methods that provide this functionality:
1. some:  Test whether some element in the array/object passes the test implemented by the provided
          function.
2. every: Test whether every element in the array/object passes the test implemented by the provided
          function.

Closes angular#263
gkalpak added a commit to gkalpak/angular.js that referenced this issue Oct 24, 2014
…Each()`)

NOTICE: This is just a POC implementation to see if there is interest for the feature.
        There are many performance improvements to be made and tests to be written (and run).
Sometimes it is useful to be able to iterate over an array/object, but abort the operation as soon
as some condition is met. This commit adds two methods that provide this functionality:
1. some:  Test whether some element in the array/object passes the test implemented by the provided
          function.
2. every: Test whether every element in the array/object passes the test implemented by the provided
          function.

Closes angular#263
gkalpak added a commit to gkalpak/angular.js that referenced this issue Oct 27, 2014
…Each()`)

NOTICE: This is just a POC implementation to see if there is interest for the feature.
        There are many performance improvements to be made and tests to be written (and run).
Sometimes it is useful to be able to iterate over an array/object, but abort the operation as soon
as some condition is met. This commit adds two methods that provide this functionality:
1. some:  Test whether some element in the array/object passes the test implemented by the provided
          function.
2. every: Test whether every element in the array/object passes the test implemented by the provided
          function.

Closes angular#263
@dragosrususv
Copy link

Shouldn't spec be written based on community needs? Just asking...
+1 for the return false, as mentioned in #9797 (someone redirected me here after I opened the other task).

I confirm I've seen this requirement in multiple projects and redirecting developers to vanilla JS is just not the way to go in a normalized world/framework.

gkalpak added a commit to gkalpak/angular.js that referenced this issue Oct 27, 2014
…Each()`)

NOTICE: This is just a POC implementation to see if there is interest for the feature.
        There are many performance improvements to be made and tests to be written (and run).
Sometimes it is useful to be able to iterate over an array/object, but abort the operation as soon
as some condition is met. This commit adds two methods that provide this functionality:
1. some:  Test whether some element in the array/object passes the test implemented by the provided
          function.
2. every: Test whether every element in the array/object passes the test implemented by the provided
          function.

Closes angular#263
gkalpak added a commit to gkalpak/angular.js that referenced this issue Oct 28, 2014
…Each()`)

NOTICE: This is just a POC implementation to see if there is interest for the feature.
        There are many performance improvements to be made and tests to be written (and run).
Sometimes it is useful to be able to iterate over an array/object, but abort the operation as soon
as some condition is met. This commit adds two methods that provide this functionality:
1. some:  Test whether some element in the array/object passes the test implemented by the provided
          function.
2. every: Test whether every element in the array/object passes the test implemented by the provided
          function.

Closes angular#263
@IgorMinar
Copy link
Contributor Author

I don't know what the reasoning TC39 had for not supporting this feature,
but I'd be surprised if they didn't discuss it.

Maybe you could do the research and see if we could revive the discussion
in TC39?

On Mon, Oct 27, 2014, 11:59 AM Dragos Rusu notifications@github.com wrote:

Shouldn't spec be written based on community needs? Just asking...
+1 for the return false, as mentioned in #9797
#9797 (someone redirected
me here after I opened the other task).

I confirm I've seen this requirement in multiple projects and redirecting
developers to vanilla JS is just not the way to go in a normalized
world/framework.


Reply to this email directly or view it on GitHub
#263 (comment).

@ghost
Copy link

ghost commented Oct 29, 2014

???

They have differently named method (.every) just for that.
forEach is specified to go over all elements unconditionally.
And I already mentioned .every in this thread. Like, do not change angular's forEach to be different from specified one, but add .every that works same as specified one.

Herby

Igor Minar wrote:

I don't know what the reasoning TC39 had for not supporting this feature,
but I'd be surprised if they didn't discuss it.

Maybe you could do the research and see if we could revive the discussion
in TC39?

On Mon, Oct 27, 2014, 11:59 AM Dragos Rusu notifications@github.com
wrote:

Shouldn't spec be written based on community needs? Just asking...
+1 for the return false, as mentioned in #9797
#9797 (someone redirected
me here after I opened the other task).

I confirm I've seen this requirement in multiple projects and
redirecting
developers to vanilla JS is just not
the way to go in a normalized
world/framework.


Reply to this email directly or view it on GitHub

#263 (comment).


Reply to this email directly or view it on GitHub
#263 (comment).

@dragosrususv
Copy link

@IgorMinar : sorry to have lead to confusion - I was referring to what @Herby is saying, that is: implement angular.every and return false there. The reason for adding it in angular would be to iterate on arrays or objects, as @gkalpak was mentioning in #9797 .

If you consider this to be useful, lets discuss with @gkalpak (I'm offering to contribute with code as well) and let's make a PR for this. Please confirm here or in #9797.

As for changing spec for .forEach - even if I would definitely advocate to that, I consider that guys at Mozilla have better visibility then I do, so if they chose to use .every, they must have had a really good reason (not compat, because people just use "return;" and a strict comparison with FALSE would have done the job).

@pkozlowski-opensource
Copy link
Member

My understanding is that we don't want to have any of those utilities in 2.0 so adding more to 1.x doesn't make much sense, IMO.

Those little utilities proved to trigger a lot of issues / and endless discussions (this one being good example) so once again - this would be -1 from my side. Let's have AngularJS focus on what it does best instead of turning it into a general purpose utility. This is just my opinion, though.

@dragosrususv
Copy link

@pkozlowski-opensource : you have your point, it is true.
Still, there's a chicken and egg problem here. If AngularJS should focus on what it's best at, then lets trash foreach - that is in vanilla js as well for arrays (everyone should have it's own utils to iterate over objects - fairly easy to do).

If again, forEach is not to be removed in 1.x or 2.0, then why would we use AngularJS forEach when we need to iterate on each item and another utility method (or default vanilla JS) when we want to stop at one point in our iteration. This does not make sense at all.

One of the reasons that jquery was made was also to normalize development (not only browser support *). So pushing back to developers to do these stuff in their own way should be either systematically addressed ("do your own utils" or "use our utils" - not BOTH).

@IgorMinar
Copy link
Contributor Author

v2 won't provide any of these helper methods. We'll rely on native apis or
modular utility libraries like lodash.

We won't add any more of these to 1.x for reasons Pawel mentioned.

And we won't remove any from 1.x because that would cause too much trouble.

I hope this explains our thinking well.

On Wed, Oct 29, 2014, 3:38 AM Dragos Rusu notifications@github.com wrote:

@pkozlowski-opensource https://github.com/pkozlowski-opensource : you
have your point, it is true.
Still, there's a chicken and egg problem here. If AngularJS should focus
on what it's best at, then lets trash foreach - that is in vanilla js as
well for arrays (everyone should have it's own utils to iterate over
objects - fairly easy to do.

If again, forEach is not to be removed in 1.x or 2.0, then why would we
use AngularJS forEach when we need to iterate on each item and another
utility method (or default vanilla JS) when we want to stop at one point in
our iteration. This does not make sense at all.

One of the reasons that jquery was made was also to normalize development
(not only browser support *). So pushing back to developers to do these
stuff in their own way should be either systematically addressed ("do your
own utils" or "use our utils" - not BOTH).


Reply to this email directly or view it on GitHub
#263 (comment).

@dragosrususv
Copy link

Sounds reasonable and systematically addressed.
I'll close #9797 .

@IgorMinar
Copy link
Contributor Author

Thanks!

On Wed, Oct 29, 2014, 8:15 AM Dragos Rusu notifications@github.com wrote:

Sounds reasonable and systematically addressed.
I'll close #9797 #9797 .


Reply to this email directly or view it on GitHub
#263 (comment).

@dragosrususv
Copy link

For people who don't want to integrate a big library into their app for EACH, check #9797 (comment) .

@IgorMinar
Copy link
Contributor Author

I'd recommend checking out https://www.npmjs.org/package/lodash.foreach instead.

@Daedalon
Copy link

Daedalon commented May 6, 2015

@IgorMinar: "v2 won't provide any of these helper methods" - does this mean that angular.forEach will not be available in v2? If it's in effect deprecated, this could be good to mention in https://docs.angularjs.org/api/ng/function/angular.forEach. Those who read that and switch to a non-deprecated solution any time before the release of v2 will have an easier migration ahead.

@dragosrususv
Copy link

Yeah. Definitely a good advise for people to start using external libraries or vanilla JS for iterations.

@danieljameswilliams
Copy link

+1

@Daedalon
Copy link

Daedalon commented Jun 4, 2015

@danieljameswilliams: As the issue has had significant discussion and a resolution with a follow-up question, would you like to clarify what do you wish to support with the +1? The original request for forEach to support break, or @IgorMinar's resolution that it won't be implemented, or the request to clarify whether forEach will be deprecated altogether?

@danieljameswilliams
Copy link

@Daedalon My "+1" was for the implementation for break; and continue;
I haven't read the whole thread (TL;DR), but I figured out last night after this post, that the benchmarking on angular.forEach vs. native for-loop is pretty slow.

@gabrielfeitosa
Copy link

+1

@Zorgatone
Copy link

+1 with function return (no exceptions)

@williamweckl
Copy link

+1

@codeofsumit
Copy link

wtf? +1!!!

@renanss
Copy link

renanss commented Apr 19, 2016

+1
Should I continue using angular.forEach? I think it's much cleaner than for-loop but someone said it much slower :o

@Narretz
Copy link
Contributor

Narretz commented Apr 19, 2016

This is a closed issue and the reasons for not supporting break are outlined in these comments: #263 (comment) and #263 (comment). Why there's no equivalent every() / some() can be read here: #263 (comment)

I'm locking this issue as discussion (or +1) in closed threads isn't leading anywhere.

@angular angular locked and limited conversation to collaborators Apr 19, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests