refactor(forEach): change to for-of with iterable

[PatrickJS]

Tests are passing. I also used git mv to preserve the git history. I changed references with the name array to iterable
closes #909

[vicb]

@gdi2290 the Travis error comes from the benchmarks. infinite_scroll uses ForEach/For and needs to be updated

[PatrickJS]

@vicb thanks I couldn't make out what the Travis error was. Give me a second

[PatrickJS]

@vicb Dart doesn't have a for-of so how do we change the directive interface for Dart users to use for-in.

[PatrickJS]

:/ I'm doing something wrong with my Dart version

dist/dart/angular2/:[hint] The value of the local variable 'item' is not used 
(/home/travis/build/angular/angular/dist/dart/angular2/lib/src/change_detection/pipes/iterable_changes.dart, line 110, col 9)

void iterateListLike(iter, fn(item)) {
  if (iter is List) {
    for (var item = 0; item < iter.length; ++item) {
      fn(iter[item]);
    }
  } else {
    for (var item in iter) {
      fn(item);
    }
  }
}

update: I found one error for iterable_changes in Dart.
update: The problem was due to optimization in array_changes.js (now iterable_changes.js)
previously

  check(collection):boolean {
    this._reset();

    var record:CollectionChangeRecord = this._itHead;
    var mayBeDirty:boolean = false;
    var index:int, item;

    if (ListWrapper.isList(collection)) {
      var list = collection;
      this._length = collection.length;

      for (index = 0; index < this._length; index++) {
        item = list[index];
        if (record === null || !looseIdentical(record.item, item)) {
          record = this._mismatch(record, item, index);
          mayBeDirty = true;
        } else if (mayBeDirty) {
          // TODO(misko): can we limit this to duplicates only?
          record = this._verifyReinsertion(record, item, index);
        }
        record = record._next;
      }
    } else {
      index = 0;
      iterateListLike(collection, (item) => {
        if (record === null || !looseIdentical(record.item, item)) {
          record = this._mismatch(record, item, index);
          mayBeDirty = true;
        } else if (mayBeDirty) {
          // TODO(misko): can we limit this to duplicates only?
          record = this._verifyReinsertion(record, item, index);
        }
        record = record._next;
        index++
      });
      this._length = index;
    }

after refactor

  check(collection):boolean {
    this._reset();

    var record:CollectionChangeRecord = this._itHead;
    var mayBeDirty:boolean = false;
    var index:int, item;

    index = 0;
    iterateListLike(collection, (item) => {
      if (record === null || !looseIdentical(record.item, item)) {
        record = this._mismatch(record, item, index);
        mayBeDirty = true;
      } else if (mayBeDirty) {
        // TODO(misko): can we limit this to duplicates only?
        record = this._verifyReinsertion(record, item, index);
      }
      record = record._next;
      index++
    });
    this._length = index;

the problem was this line sine var item hoist as undefined some people show it explicitly but was more subtle was the combination of multiple variable declaration using , on the same line rather than new line

    var index:int, item;

which is now

    var index:int = 0;

[PatrickJS]

@mhevery I rebased and fixed the merge conflicts

[PatrickJS]

is there a better way to rebase on github? I always force push up changes after rebase but that also blows up any commits that were made in the files previously :/

[PatrickJS]

I also fixed the build in this commit foreach_spec.js#L127

[mhevery]

In general I like this commit. You did make the change detection slower however. Those parts need to be reverted, but the rest can go. Please ping this thread once ready.

[PatrickJS]

The commit was squash without other changes. I also preserved the git history during renaming. There needs to be Iterator facade next and then a perf PR to avoid polymorphic(or was it megamorphic) code for when it's an iterable

[caitp]

there are only a handful of iterables most people are going to use, it's not likely to result in IC misses if the path is only taken when the object is known statically to be iterable (would not necessarily "replace" Array change detection with Iterable change detection, though.)

[PatrickJS]

gg yet another rebase and git mv. I have to update the TodoMVC example to use For rather than Foreach
update: I made a helper script so rebase isn't a pain anymore https://github.com/gdi2290/git-mv

[PatrickJS]

@mhevery PR is all green now with correct rebase and renames tracked by git

[PatrickJS]

@mhevery I rebase'd Master again so everything is green. If you have any suggestions or changes feel free to ping me again about it

[mhevery]

b61b8d6

[angular-automatic-lock-bot]

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

_{This action has been performed automatically by a bot.}