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

Preferred method of integer casting? #18

Closed
ajacksified opened this issue Nov 7, 2012 · 9 comments
Closed

Preferred method of integer casting? #18

ajacksified opened this issue Nov 7, 2012 · 9 comments

Comments

@ajacksified
Copy link
Contributor

It's a small thing, but we should probably be consistent here, too. What's the best way to cast to integers? I have the habit of bitshifting by 0 "15" >> 0; it's one of the more performant ways to do it, and it's concise, but it also isn't super obvious.

@hshoff
Copy link
Member

hshoff commented Nov 7, 2012

I always do +'15', open to changing this if there's a more preferred way.

see Numbers: https://github.com/airbnb/javascript#type-coercion

@hshoff
Copy link
Member

hshoff commented Nov 7, 2012

Here's a jsPerf Coercion vs. Casting vs. Bitshift

Looks like casting with parseInt() is the fastest.

@ajacksified
Copy link
Contributor Author

parseInt looks good.

@ssorallen
Copy link
Contributor

I didn't expect bitshifting to be so much slower than parseInt.

I always use parseInt because it's explicit about what you're doing. +'1' and '1' >> 0 work, but they're tricks. If you are parsing an integer, you can't get more explicit than a function called "parseInt".

@ssorallen
Copy link
Contributor

If we go with parseInt, we should enforce always including a radix.

// Bad
parseInt('123');

// Good
parseInt('123', 10);

@ajacksified
Copy link
Contributor Author

Took a look with other browsers- bitshift is drastically better on FF and IE. Not sure if buggy test result.

@hshoff
Copy link
Member

hshoff commented Nov 7, 2012

Woah, what's Webkit got against bitshift?

I vote we go with parseInt because it's explicit.

But it should be allowed to do bitshifting if you need to performance boost on lots of strings to ints. Maybe just leave a comment why you think you need to bitshift over being explicit?

@clizzin
Copy link

clizzin commented Nov 7, 2012

👍 for parseInt in general for clarity, and bitshifting with an explanatory comment when necessary for performance — although IMO it takes a lot to justify performance > clarity. But that's just bikeshedding.

Also 👍 for including a radix when calling parseInt. It is ugly and verbose, but it's a hell of a lot better than a prefixed 0 ruining your day.

@reissbaker
Copy link
Contributor

👍 for parseInt with a radix. Bitshifts aren't very clear, and for our types of applications VM execution speed is a non-concern.

But it should be allowed to do bitshifting if you need to performance boost on lots of strings to ints.

Fair, if you have profiling that proves parseInt is a major performance bottleneck. But optimizing without profiling is a bad idea.

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

No branches or pull requests

5 participants