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

For loops don't work consistently for strings #1814

Closed
joewhite opened this Issue Oct 30, 2011 · 3 comments

Comments

Projects
None yet
4 participants
@joewhite

joewhite commented Oct 30, 2011

I found myself needing to loop over the characters in a string, so I tried a for loop:

for char, index in line:
    # ...

It turns out this isn't actually supported, but I couldn't tell because it worked just fine. CoffeeScript happily compiled my loop to:

for (index = 0, _len2 = line.length; index < _len2; index++) {
  char = line[index];
  // ...

But as I later learned, JavaScript strings haven't always supported indexers -- and sometimes still don't; if you run this code in IE, and your page does not have an HTML5 doctype declaration, then line[x] will return undefined. (If you do have the HTML5 doctype, then this code works just fine in IE. Go figure.)

For reference, it looks like the correct way to loop over the characters in a string (which, unfortunately, requires remembering the difference between those wretched .. and ... operators) is:

for index in [0...line.length]:
    char = line.charAt index
    # ...

It would be nice if CoffeeScript didn't make it so easy to generate code that looks fine in modern browsers but that will fail in older browsers. Perhaps the documentation should clearly and loudly call out the fact that for isn't meant for use with strings (though that would be a pity; the "correct" alternative is a lot uglier). Perhaps the compiler should emit guard code around for loops to throw a "hey, this isn't supported" error if the thing being iterated over is a string. Perhaps the compiler should emit code that works correctly even if you do pass in a string. Other ideas?

@michaelficarra

This comment has been minimized.

Show comment
Hide comment
@michaelficarra

michaelficarra Oct 30, 2011

Collaborator

It turns out this isn't actually supported

correct

JavaScript strings haven't always supported indexers

correct

it looks like the correct way to loop over the characters in a string [...] is: [...]

for char, index in str.split ''

Perhaps the documentation should clearly and loudly call out the fact that for isn't meant for use with strings

perhaps

Perhaps the compiler should emit guard code around for loops

no, it shouldn't: very unnecessary overhead

Perhaps the compiler should emit code that works correctly even if you do pass in a string

no, for the same reason

Collaborator

michaelficarra commented Oct 30, 2011

It turns out this isn't actually supported

correct

JavaScript strings haven't always supported indexers

correct

it looks like the correct way to loop over the characters in a string [...] is: [...]

for char, index in str.split ''

Perhaps the documentation should clearly and loudly call out the fact that for isn't meant for use with strings

perhaps

Perhaps the compiler should emit guard code around for loops

no, it shouldn't: very unnecessary overhead

Perhaps the compiler should emit code that works correctly even if you do pass in a string

no, for the same reason

@jashkenas

This comment has been minimized.

Show comment
Hide comment
@jashkenas

jashkenas Oct 31, 2011

Owner

I'm afraid that JavaScript has a great number of pitfalls and footguns in all areas ... and the CoffeeScript documentation can't possibly enumerate them all. If it did, it would be many hundreds of pages longer than it currently is. I'm sorry that this particular one bit you, but I'm afraid there's not much we should do about it.

Owner

jashkenas commented Oct 31, 2011

I'm afraid that JavaScript has a great number of pitfalls and footguns in all areas ... and the CoffeeScript documentation can't possibly enumerate them all. If it did, it would be many hundreds of pages longer than it currently is. I'm sorry that this particular one bit you, but I'm afraid there's not much we should do about it.

@jashkenas jashkenas closed this Oct 31, 2011

@matanster

This comment has been minimized.

Show comment
Hide comment
@matanster

matanster Dec 24, 2013

@joewhite have you maybe compared performance of splitting the string for the loop v.s. looping the plain string and using charAt?

matanster commented Dec 24, 2013

@joewhite have you maybe compared performance of splitting the string for the loop v.s. looping the plain string and using charAt?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment