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

Changed for...in to for...of #59

Closed
wants to merge 1 commit into from
Closed

Conversation

andrasf
Copy link

@andrasf andrasf commented May 4, 2022

I was trying to use the library, but had fatal exceptions upon application startup.

It turned out I was using an array extension that would be hard to remove, that added a method to Array.prototype. This method shows up if an array is used with for (let i in array), but does not show up if used with for (let i of Object.keys(array)), causing side effects.

This made country-coder to crash when calling methods on arrays, eg. charAt, when it did not expect to handle anything other than strings.

@andrasf andrasf changed the title Changed for...in for for...of Changed for...in to for...of May 4, 2022
@bhousel
Copy link
Collaborator

bhousel commented May 4, 2022

Whoa.. I kind of think both of these array iteration methods are pretty weird.

for..in loops over iterable keys, which in an array happens to usually (but not definitely) contain the array indexes.
for..of at least loops over the array values, but then - to use Object.keys to turn them back into keys is .. really weird.

Thanks for raising this issue, but I think I'd feel better just rewriting all these for..in loops to just use either normal for loops or normal array iterators.

@andrasf
Copy link
Author

andrasf commented May 4, 2022

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants