search

WebDevTmas / moment-round

Plugin for momentjs to round time in dates.
Other
47 stars 75 forks source link

Use for loop instead of for in #8

Open lukas-shawford opened 8 years ago

lukas-shawford commented 8 years ago

I see a couple pull requests fixing the same issue with array iteration when the array prototype has been extended (#2, #7), so I thought I'd open another one :-)

I think this fix is simplest - just use a regular for loop instead of for..in. The trouble with for..in is it doesn't guarantee iteration order, and it also picks up inherited properties. For these reasons, it is generally not recommended to use for..in for iterating over an array.

I see the other pull requests addressed this issue either by either checking the type to ensure it's not a function, or using hasOwnProperty, but I think this method is the simplest - it's what a for loop is for!

viki53 commented 8 years ago

:+1:

I was just about to do the same PR, this was really annoying and a waste of time to debug.