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

Number formatting with “d” code – needs explanation & example fix #42

Closed
yucca42 opened this issue Jun 21, 2011 · 3 comments
Closed

Comments

@yucca42
Copy link

yucca42 commented Jun 21, 2011

The description of ”d” number formatting code as ”for decimal digits” sounds very odd. It seems that the format code ”d” really means using the non-localized JavaScript number format, just with left-padding the string with zeros to get a minimum width specified by number after the format character ”d”.

So Globalize.format( 123.45, "d" ) yields 123.45 (not 123 as currently in the documentation). Maybe the documentation is meant to use 123 as the first parameter, on the grounds that the ”d” format is really meant for integers only, isn’t it? Moreover, it should perhaps be emphasized that the result is not localized, e.g. Globalize.format( 1234, "d") yields 1234 independently of locale, not a localized presentation with a thousands separator (1,234 or 1 234 or something like that).

@rdworth
Copy link
Contributor

rdworth commented Jun 21, 2011

It seems like it doesn't work as expected (as documented) when a float is provided versus when an int is provided. Assuming it originated from .Net (which is where the culture info in Globalize originates from), it may not be intended for use with floats at all. See http://msdn.microsoft.com/en-us/library/dwhawy9k.aspx#DFormatString "This format is supported only for integral types."

It also seems to not handle negative numbers:

Globalize.format( -1234, "d7" ) // expected: "-0001234", actual "-1234"

thought the .Net version does.

I would not except "d" to provide grouping as that is provided by "n". So if you want "1,234" or "1 234" then call format with "n" but if you want "1234", call format with "d". Or for "01234", "d5". I think you're right in that it doesn't seem that will ever be affected by a culture, as it excludes grouping and thousands and decimal (point) separators.

rdworth added a commit that referenced this issue Jun 21, 2011
@rdworth
Copy link
Contributor

rdworth commented Jun 21, 2011

Added (failing) unit test in 47fc9e3 for negative integer with expected padding 0s after the negative sign.

@yucca42
Copy link
Author

yucca42 commented Jun 21, 2011

I think this is a documentation issue. Using plain “d” is rather pointless, as with it, format() does just the job of Number.toString(). With a numeric specifier, it’s potentially useful when you need fixed-width integers left padded with zeroes (though I wonder how common the need is, except for dates and times, for which it can be done another way).

I’d suggest changing the documentation to something like the following:

• d for number in non-localized format (as per Number.toString()) padded with zeros on the left

In the prose after the bulleted list, change
“The number determines how many decimal places to display for all the format types except decimal, for which it means the minimum number of digits to display, zero padding it if necessary.”
to
“The number determines how many decimal places to display for all the format types except ‘d’, for which it means the minimum number of characters to display, zero padding it on the left if necessary.”

Oh, wait... for a negative number, we always get the result as per Number.toString(), without zero padding, apparently because the code first takes the absolute value, then creates the formatted string (possibly with leading zeros), and finally does number = -number if the value was negative. And this destroys the zero padding.

@yucca42 yucca42 closed this as completed Jun 21, 2011
@yucca42 yucca42 reopened this Jun 21, 2011
rdworth added a commit that referenced this issue Jun 22, 2011
… when 'd' format specified. Partial fix for issue #42
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

2 participants