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

Added string formatting options to ToQuantity #220

Closed
wants to merge 5 commits into from

Conversation

dealdiane
Copy link

No description provided.

dealdiane pushed a commit to dealdiane/Humanizer that referenced this pull request Apr 15, 2014
@dealdiane
Copy link
Author

readme probably needs updating as well?

@MehdiK
Copy link
Member

MehdiK commented Apr 16, 2014

I like it. Good work :)

Yes, we need to include this in the readme or no one's ever going to know about it.

@dealdiane
Copy link
Author

Done. Please proofread though.

@MehdiK
Copy link
Member

MehdiK commented Apr 16, 2014

Thanks for adding the readme bit.

There are three failing tests. Also your fork is quite a few commits behind the upstream. Please fix the tests and rebase your work. Thanks.


```C#
"dollar".ToQuantity(2, "C0", new CultureInfo("en-US")) => "$2 dollars"
"dollar".ToQuantity(2, "C2", new CultureInfo("en-US")) => "$2.00 dollars"
Copy link
Member

Choose a reason for hiding this comment

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

This is pretty cool. Thanks for the great addition.

@dealdiane
Copy link
Author

Is it the tests I have added that are failing?

@MehdiK
Copy link
Member

MehdiK commented Apr 16, 2014

Yes.

Test 'Humanizer.Tests.ToQuantityTests.ToQuantityWordsWithCustomCultureFormatting(word: "case", quantity: 1234567, format: "N2", cultureCode: "de-CH", expected: "1 234 567,00 cases")' failed:
Assert.Equal() Failure
Position: First difference is at position 1
Expected: 1 234 567,00 cases
Actual: 1'234'567.00 cases
ToQuantityTests.cs(116,0): at Humanizer.Tests.ToQuantityTests.ToQuantityWordsWithCustomCultureFormatting(String word, Int32 quantity, String format, String cultureCode, String expected)

Test 'Humanizer.Tests.ToQuantityTests.ToQuantityWordsWithCustomCultureFormatting(word: "case", quantity: 1234567, format: "N0", cultureCode: "de-CH", expected: "1 234 567 cases")' failed:
Assert.Equal() Failure
Position: First difference is at position 1
Expected: 1 234 567 cases
Actual: 1'234'567 cases
ToQuantityTests.cs(116,0): at Humanizer.Tests.ToQuantityTests.ToQuantityWordsWithCustomCultureFormatting(String word, Int32 quantity, String format, String cultureCode, String expected)

Test 'Humanizer.Tests.ApiApprover.PublicApiApprovalTest.approve_public_api' failed: ApprovalTests.Core.Exceptions.ApprovalMismatchException : Failed Approval: Received file c:_mine\Code\Humanizer\src\Humanizer.Tests\ApiApprover\PublicApiApprovalTest.approve_public_api.received.txt does not match approved file c:_mine\Code\Humanizer\src\Humanizer.Tests\ApiApprover\PublicApiApprovalTest.approve_public_api.approved.txt.

@dealdiane
Copy link
Author

Just did a rebase.
That is weird. All my tests are green except for that one I just fixed now.

Can you by any chance run this piece of code on your machine and paste your output here?

var number = 1234567;
number.ToString("N0", new CultureInfo("en-US")).ToList().ForEach(x => Console.Write((int)x + ":" + x + "-"));
number.ToString("N2", new CultureInfo("en-US")).ToList().ForEach(x => Console.Write((int)x + ":" + x + "-"));
number.ToString("N0", new CultureInfo("de-CH")).ToList().ForEach(x => Console.Write((int)x + ":" + x + "-"));
number.ToString("N2", new CultureInfo("de-CH")).ToList().ForEach(x => Console.Write((int)x + ":" + x + "-"));

49:1-44:,-50:2-51:3-52:4-44:,-53:5-54:6-55:7-
49:1-44:,-50:2-51:3-52:4-44:,-53:5-54:6-55:7-46:.-48:0-48:0-
49:1-160: -50:2-51:3-52:4-160: -53:5-54:6-55:7-
49:1-160: -50:2-51:3-52:4-160: -53:5-54:6-55:7-44:,-48:0-48:0-

@MehdiK
Copy link
Member

MehdiK commented Apr 16, 2014

Sure. Here it is:

49:1-44:,-50:2-51:3-52:4-44:,-53:5-54:6-55:7-
49:1-44:,-50:2-51:3-52:4-44:,-53:5-54:6-55:7-46:.-48:0-48:0-
49:1-39:'-50:2-51:3-52:4-39:'-53:5-54:6-55:7-
49:1-39:'-50:2-51:3-52:4-39:'-53:5-54:6-55:7-46:.-48:0-48:0-

These quotation marks are strange!

@dealdiane
Copy link
Author

Yeah, seems strange that the same culture is giving different results. May I know what version of .NET you're running the tests on and on what platform?

I've made a slight change in the same code above. Was wondering if you can run it again?

var number = 1234567;
number.ToString("N0", new CultureInfo("de-CH", true)).ToList().ForEach(x => Console.Write((int)x + ":" + x + "-"));
number.ToString("N2", new CultureInfo("de-CH", false)).ToList().ForEach(x => Console.Write((int)x + ":" + x + "-"));
number.ToString("N0", CultureInfo.GetCultureInfo("de-CH")).ToList().ForEach(x => Console.Write((int)x + ":" + x + "-"));
number.ToString("N2", CultureInfo.GetCultureInfo("de-CH")).ToList().ForEach(x => Console.Write((int)x + ":" + x + "-"));

@MehdiK
Copy link
Member

MehdiK commented Apr 16, 2014

49:1-39:'-50:2-51:3-52:4-39:'-53:5-54:6-55:7-
49:1-39:'-50:2-51:3-52:4-39:'-53:5-54:6-55:7-46:.-48:0-48:0-
49:1-39:'-50:2-51:3-52:4-39:'-53:5-54:6-55:7-
49:1-39:'-50:2-51:3-52:4-39:'-53:5-54:6-55:7-46:.-48:0-48:0-

On Wed, Apr 16, 2014 at 1:04 PM, opiants [email protected] wrote:

Yeah, seems strange that the same culture is giving different results. May
I know what version of .NET you're running the tests on and on what
platform?

I've made a slight change in the same code above. Was wondering if you can
run it again?

var number = 1234567;number.ToString("N0", new CultureInfo("de-CH", true)).ToList().ForEach(x => Console.Write((int)x + ":" + x + "-"));number.ToString("N2", new CultureInfo("de-CH", false)).ToList().ForEach(x => Console.Write((int)x + ":" + x + "-"));number.ToString("N0", CultureInfo.GetCultureInfo("de-CH")).ToList().ForEach(x => Console.Write((int)x + ":" + x + "-"));number.ToString("N2", CultureInfo.GetCultureInfo("de-CH")).ToList().ForEach(x => Console.Write((int)x + ":" + x + "-"));

Reply to this email directly or view it on GitHubhttps://github.com//pull/220#issuecomment-40575402
.

@MehdiK
Copy link
Member

MehdiK commented Apr 16, 2014

I created the test in the Humanizer test project.

On Wed, Apr 16, 2014 at 1:06 PM, Mehdi Khalili [email protected] wrote:

49:1-39:'-50:2-51:3-52:4-39:'-53:5-54:6-55:7-
49:1-39:'-50:2-51:3-52:4-39:'-53:5-54:6-55:7-46:.-48:0-48:0-
49:1-39:'-50:2-51:3-52:4-39:'-53:5-54:6-55:7-
49:1-39:'-50:2-51:3-52:4-39:'-53:5-54:6-55:7-46:.-48:0-48:0-

On Wed, Apr 16, 2014 at 1:04 PM, opiants [email protected] wrote:

Yeah, seems strange that the same culture is giving different results.
May I know what version of .NET you're running the tests on and on what
platform?

I've made a slight change in the same code above. Was wondering if you
can run it again?

var number = 1234567;number.ToString("N0", new CultureInfo("de-CH", true)).ToList().ForEach(x => Console.Write((int)x + ":" + x + "-"));number.ToString("N2", new CultureInfo("de-CH", false)).ToList().ForEach(x => Console.Write((int)x + ":" + x + "-"));number.ToString("N0", CultureInfo.GetCultureInfo("de-CH")).ToList().ForEach(x => Console.Write((int)x + ":" + x + "-"));number.ToString("N2", CultureInfo.GetCultureInfo("de-CH")).ToList().ForEach(x => Console.Write((int)x + ":" + x + "-"));

Reply to this email directly or view it on GitHubhttps://github.com//pull/220#issuecomment-40575402
.

@dealdiane
Copy link
Author

Strangely enough, running the tests on <= 3.5 will return the same format you are getting but on > 4.0 it returns what I'm getting. I don't think the framework version is the problem though cos the project's target framework is 4.0 and should be the same for everyone? But surely it has something to do with the framework but just can't figure out what and why.

What do you suggest we do going forward?

@dealdiane
Copy link
Author

They've updated the number formats on newer OSes and frameworks and it seems that they won't bother updating the number formats to make it more consistent.

https://connect.microsoft.com/VisualStudio/feedback/details/772498/inconsistent-formatting-behavior-of-double-tostring-in-net-3-5-vs-4-x
http://social.msdn.microsoft.com/Forums/vstudio/en-US/3ac50c1a-4f0b-450f-82f2-686a89766391/suddenly-no-more-group-seperator-for-culture-ptpt-in-clr-4-win8?forum=netfxbcl

I'll update the tests so it'll use a more consistent culture code and stop it from failing.

@MehdiK
Copy link
Member

MehdiK commented Apr 17, 2014

Sorry I couldn't this yesterday. Had a crazy day! That was an interesting issue & good find.

Thanks for the great work. Merged now.

@MehdiK MehdiK closed this Apr 17, 2014
MehdiK added a commit that referenced this pull request Apr 19, 2014
@MehdiK
Copy link
Member

MehdiK commented Apr 19, 2014

Thanks for the contribution. This is now available on NuGet as v1.23.1.

clearpath pushed a commit to clearpath/Humanizer that referenced this pull request Apr 22, 2014
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

Successfully merging this pull request may close these issues.

2 participants