humanize icon indicating copy to clipboard operation
humanize copied to clipboard

Added conversion to millions, billions, etc

Open sandro-pasquali opened this issue 12 years ago • 1 comments

sandro-pasquali avatar Jul 19 '12 22:07 sandro-pasquali

Thanks for the pull request. Unfortunately, I don't really understand the code when I skim it, and for a function of this nature, I personally think it should be obvious instead of convoluted. I briefly looked at the gist, and it seems like the massive regex is inserting commas every 3 digits (I'm not even completely sure about that). That seems like a very roundabout way of doing what is desired.

The way I would structure this code instead would be: loop on (if n < 1000, n/1000) and count the number of times this happens to determine what the string part should be, maybe with an upper limit since our max is trillion. Instead of rounding automatically, I would prefer to see a flag that specifies the number of significant digits, defaulting to 3 (since we're dividing by 1000). Finally, I would appreciate it if you also added unit tests for this, as the entire system has been setup at great lengths.

taijinlee avatar Jul 19 '12 22:07 taijinlee