Skip to content
This repository has been archived by the owner on Oct 12, 2023. It is now read-only.

Fixed negative numbers issue + slightly improved performance #13

Merged
merged 6 commits into from
Jun 14, 2015
Merged

Fixed negative numbers issue + slightly improved performance #13

merged 6 commits into from
Jun 14, 2015

Conversation

pziemkowski
Copy link

Hello,

Negative numbers were converted to their absolute values since dash sign was present (and thus removed). It may look hacky but as far as I know it is the most efficient way to check whether string is a number since NaN !== NaN.

I also changed first character conversion to uppercase or lowercase. It uses substr function instead of regexp which makes it a lot faster (jsPerf). I had a huge object to convert and humps began to hurt.

Tests updated accordingly.

@domchristie
Copy link
Owner

Thanks for this, great work!

I have a couple of questions which I’ll ask inline…

Thanks again

@@ -0,0 +1,2 @@
.idea
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Out of interest, what is .idea?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's Jetbrains project's configuration directory (you know - WebStorm, PHPStorm, PyCharm, IntelliJ Idea, etc). Lot's of people have this and push it by mistake. I thought it would be a good idea (pun not intended) to exclude it to be sure.

@pziemkowski
Copy link
Author

So, what do we do with this PR?

@domchristie
Copy link
Owner

@patrykziem sorry I’ve not got back to you on this one, and thanks for the reminder!

Re: isNan: it’d be great if you could implement a _isNan function with a comment, to solve that problem, then I’ll get this merged.

Thanks again

@pziemkowski
Copy link
Author

@domchristie done. Is this what you had in mind?

@pziemkowski
Copy link
Author

I changed it to _isNumerical, but I had to change the code you proposed a bit. Since its not about comparing string to a number that gives as the answer. It's turning string into number and then checking whether its NaN.

@domchristie
Copy link
Owner

Great, thanks!

domchristie added a commit that referenced this pull request Jun 14, 2015
Fixed negative numbers issue + slightly improved performance
@domchristie domchristie merged commit ef3cfdf into domchristie:master Jun 14, 2015
@domchristie
Copy link
Owner

Released in 0.6.0

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

Successfully merging this pull request may close these issues.

2 participants