Use i instead of indexOf function call for splice

[adamsea]

This is a basic optimization to using the current loop iteration variable i instead of the indexOf lookup for splicing the array of cached events. A micro-optimization for small arrays, but will scale much better for large arrays of events.

[akre54]

Ah. Yeah I'm not sure why, but I think we had this in place so you could remove handlers out of order. Tests seem to pass without it, so it should be fine. Thanks.

[akre54]

It was left over from before 2226d3038bc045ada9ac6d23cb900add77286709 when we were chaining underscore methods and couldn't rely on i being right. Thanks for the catch.

[adamsea]

Ah yes, that makes sense! Glad to help.

[akre54]

Actually. Now that I think about this I'm not sure this is right. Because splice mutates the _domEvents array the i will point to the wrong index if more than one handler matches the undelegate args. I'll look into it tomorrow to be certain.

(I don't think I've ever used undelegate directly, I've never run into this...)

[adamsea]

Yes, you are correct - it will reindex the array, which you'll notice if you have two handlers in sequential order in the array set for removal. It may just be a matter of decrementing i after the splice: http://codepen.io/adamsea/pen/qdjWNB?editors=001

[akre54]

Yeah that's probably a smarter solution. We also wouldn't need to clone the events array if we did this

On Tue, Jun 9, 2015, 17:18 Eric Adams notifications@github.com wrote:

Yes, you are correct - it will reindex the array, which you'll notice if you have two handlers in sequential order in the array set for removal. It may just be a matter of decrementing i after the splice: http://codepen.io/adamsea/pen/qdjWNB?editors=001

— Reply to this email directly or view it on GitHub https://github.com/akre54/Backbone.NativeView/pull/17#issuecomment-110506849 .

[adamsea]

So there was a more obvious solution that I had missed - just run the undelegate/splice in a decrementing while loop to avoid reindexing issues. Tests pass, and sequential events will be removed. :)

[akre54]

Awesome. Send that pull!

On Tue, Jun 9, 2015, 18:40 Eric Adams notifications@github.com wrote:

So there was a more obvious solution that I had missed - just run the undelegate/splice in a decrementing while loop to avoid reindexing issues. Tests pass, and sequential events will be removed. :)

— Reply to this email directly or view it on GitHub https://github.com/akre54/Backbone.NativeView/pull/17#issuecomment-110525018 .

[adamsea]

Sent! #18