Yup. My question wasn't about the code or tests. I understand them and why
they pass.  My question was about forEach needing to handle the same
parameters as Array.prototype.forEach. I see your argument and am convinced
again.
On Nov 16, 2013 7:00 AM, "Adam Hyland" <notifications@github.com> wrote:

> In apps/sms/js/drafts.js:
>
> > +  });
> > +
> > +  /**
> > +  * forEach
> > +  *
> > +  * Iterate over the entire list of Draft objects
> > +  * and call the callback on each.
> > +  *
> > +  * @param  {Function} callback to call on each draft.
> > +  *
> > +  * @return {Undefined} void return.
> > +  */
> > +  DraftList.prototype.forEach = function(callback) {
> > +    var self = priv.get(this);
> > +    for (var i = 0; i < self.drafts.length; i++) {
> > +      callback.call(self.drafts[i]);
>
> Those tests below (and the old ones) pass for me using this fill for
> forEach. I think if we're going to call the method 'forEach' we should
> supply the same functionality forEach has for arrays. In this case (aside
> from the optional this argument), every test which expects a single draft
> object per iteration should pass (because the array element is the first
> argument). Alternately, since arr is actually an array, we don't need to
> fill it in:
>
> DraftList.prototype.forEach = function(callback, scope) {
>   var arr = Array.prototype.slice.call(priv.get(this).drafts, 0);
>   Array.prototype.forEach.call(arr, callback, scope);
> };
>
> Now we're back to your original implementation but because we make a copy
> of the array, it doesn't matter what we expose as arguments to the
> callback, the DraftList can't be mutated.
>
> —
> Reply to this email directly or view it on GitHub<https://github.com/mozilla-b2g/gaia/pull/13756/files#r7709110>
> .
>