Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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
Member

@IgorMinar IgorMinar commented Feb 5, 2011

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

@vojtajina vojtajina commented May 25, 2011

+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
Member Author

@IgorMinar IgorMinar commented May 25, 2011

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

@vojtajina vojtajina commented May 26, 2011

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

@IgorMinar
Copy link
Member Author

@IgorMinar IgorMinar commented May 26, 2011

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

@JensRantil JensRantil commented Aug 30, 2012

+1 Needed this today.

I like your proposed solution, @IgorMinar.

@IgorMinar
Copy link
Member Author

@IgorMinar IgorMinar commented Aug 30, 2012

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. :-/

@IgorMinar IgorMinar closed this Aug 30, 2012
@ghost
Copy link

@ghost 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 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 ProLoser commented Sep 7, 2012

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

@jamie-pate
Copy link

@jamie-pate jamie-pate commented Sep 7, 2012

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
Member

@mhevery 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 [email protected] 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
Member Author

@IgorMinar IgorMinar commented Sep 7, 2012

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

@ghost
Copy link

@ghost ghost commented Sep 14, 2012

@IgorMinar I said some or every which has.

@saurabhnanda
Copy link

@saurabhnanda saurabhnanda commented Feb 12, 2013

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

@PhantomRay PhantomRay commented Mar 31, 2013

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

Or as saurabhnanda said, add another function.

@pocesar
Copy link
Contributor

@pocesar pocesar commented Apr 17, 2013

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

@patrickkettner
Copy link

@patrickkettner patrickkettner commented Apr 17, 2013

Perhaps angular could add an .every() method?

@cburgdorf
Copy link
Contributor

@cburgdorf cburgdorf commented Apr 17, 2013

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

@saurabhnanda saurabhnanda commented Apr 17, 2013

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

On Wed, Apr 17, 2013 at 12:01 PM, Christoph Burgdorf <
[email protected]> 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

@patrickkettner patrickkettner commented Apr 17, 2013

@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

@saurabhnanda saurabhnanda commented Apr 17, 2013

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
[email protected]:

@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

@patrickkettner patrickkettner commented Apr 17, 2013

@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

@cburgdorf cburgdorf commented Apr 17, 2013

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 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

@patrickkettner patrickkettner commented Apr 17, 2013

@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

@jamie-pate jamie-pate commented Apr 17, 2013

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

@cburgdorf cburgdorf commented Apr 17, 2013

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

@ghost
Copy link

@ghost 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

@cburgdorf cburgdorf commented Apr 18, 2013

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

@dragosrususv dragosrususv commented Oct 27, 2014

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
Member Author

@IgorMinar IgorMinar commented Oct 29, 2014

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 [email protected] 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 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 [email protected]
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

@dragosrususv dragosrususv commented Oct 29, 2014

@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

@pkozlowski-opensource pkozlowski-opensource commented Oct 29, 2014

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

@dragosrususv dragosrususv commented Oct 29, 2014

@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
Member Author

@IgorMinar IgorMinar commented Oct 29, 2014

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 [email protected] 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

@dragosrususv dragosrususv commented Oct 29, 2014

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

@IgorMinar
Copy link
Member Author

@IgorMinar IgorMinar commented Oct 29, 2014

Thanks!

On Wed, Oct 29, 2014, 8:15 AM Dragos Rusu [email protected] 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

@dragosrususv dragosrususv commented Oct 30, 2014

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

@IgorMinar
Copy link
Member Author

@IgorMinar IgorMinar commented Nov 11, 2014

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

@Daedalon
Copy link

@Daedalon 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

@dragosrususv dragosrususv commented May 26, 2015

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

@danieljameswilliams
Copy link

@danieljameswilliams danieljameswilliams commented Jun 3, 2015

+1

@Daedalon
Copy link

@Daedalon 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

@danieljameswilliams danieljameswilliams commented Jun 4, 2015

@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

@gabrielfeitosa gabrielfeitosa commented Jun 17, 2015

+1

@Zorgatone
Copy link

@Zorgatone Zorgatone commented Aug 26, 2015

+1 with function return (no exceptions)

@williamweckl
Copy link

@williamweckl williamweckl commented Nov 19, 2015

+1

@codeofsumit
Copy link

@codeofsumit codeofsumit commented Jan 26, 2016

wtf? +1!!!

@renanss
Copy link

@renanss 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 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
Linked pull requests

Successfully merging a pull request may close this issue.

None yet