A failing test in last commit: local culture & invariant culture about csvhelper

[vcaraulean]

If I revert the changes in CsvReader.cs line 412 then all tests are green on my PC.

Is there any particular issue why you're forcing conversion being do in Invariant Culture?

[JoshClose]

Hmm... That might be an issue with the test itself. Meaning, it's using DateTime.Now.ToString() in your culture, and then trying to convert it using invariant culture. Since invariant is a form of english, it must just work for me. I can fix the test. Is that the only test that has issues?

This brings up a good point though. I think there should probably be a configuration setting for culture that defaults to invariant. So if the user is in a different culture and is reading text that's in their culture, they can explicitly set the culture, and it will convert it correctly. Same with writing.

[vcaraulean]

Yes, fixing the test is an easy move, just set the current thread's culture
to invariant. And that was only one test failing...

If you have seen or heard about people having need to configure it then
having a configurable option would be nice. If you go that
route, the default value (local or invariant) should be carefully
considered. In my opinion the local should be the default one. If I'm doing
a simple DateTime.Now.ToString() without specifying in my code the culture
why should I say to CsvHelper to use a specific culture when parsing a
value?

For me, if I change in both places ConvertFromInvariantString to
ConvertFromString all tests are green without any explicit Culture
manipulation.

That's why I've been asking why have you changed converter to use
ConvertFromInvariantString? Any specific reasoning?

On Mon, Jul 25, 2011 at 9:21 PM, JoshClose <
[email protected]>wrote:

Hmm... That might be an issue with the test itself. Meaning, it's using
DateTime.Now.ToString() in your culture, and then trying to convert it using
invariant culture. Since invariant is a form of english, it must just work
for me. I can fix the test. Is that the only test that has issues?

This brings up a good point though. I think there should probably be a
configuration setting for culture that defaults to invariant. So if the user
is in a different culture and is reading text that's in their culture, they
can explicitly set the culting, and it will convert it correctly. Same with
writing.

Reply to this email directly or view it on GitHub:
#16 (comment)

[JoshClose]

The reason it's there is because of a suggestion from someone. I haven't done much with culture before and hadn't even really thought about it. Now there are many people with a different culture than mine, so I need to account for this better. I will need to put some unit tests in to make sure that it works across cultures. The reason it's there is pretty much because of a lack of understanding of cultures.

I think your suggestion of using the local culture by default is correct and I will put in that config.

[vcaraulean]

Until proper configuration is done should I change code back to
ConvertFromString in CsvReader?
I have another commit ready for a pull request, they can go together...

On Tue, Jul 26, 2011 at 4:07 PM, JoshClose <
[email protected]>wrote:

The reason it's there is because of a suggestion from someone. I haven't
done much with culture before and hadn't even really thought about it. Now
there are many people with a different culture than mine, so I need to
account for this better. I will need to put some unit tests in to make sure
that it works across cultures. The reason it's there is pretty much because
of a lack of understanding of cultures.

I think your suggestion of using the local culture by default is correct
and I will put in that config.

Reply to this email directly or view it on GitHub:
#16 (comment)