CSVField: new member function try_parse_decimal() to specify one or more decimal symbols

[wilfz]

I much appreciate your library. But in most european countries the decimal separator is comma ',' and not dot ','. The trivial solution would be to add just "case ',':" in internals::data_type(). But that might break existing applications that rely on '.' and nothing else as decimal symbol. On the other hand a static member "string _decimalpoints;" in CSVField would do the thing but changing that at runtime would break multithreading safety. So I added another member function try_parse_decimal()similar to try_parse_hex()except that it also sets the private members _type and value.

[vincentlaucsb]

Thanks for your PR and for bringing the comma vs dot issue to my attention. I'll take a closer look at this when I have time.

[wilfz]

just added the case '+': to try_convert() decimal(), too

[vincentlaucsb]

First off, I appreciate you making a PR to improve this library that minimizes breaking changes.

I agree the CSVField methods should be more tolerant of non-imperial decimal number representations.

However, here's the changes I would like to see:

• I would use a char and not a string to specify the decimal symbol. I do not think there is a good use case for being able to have two possible decimal characters in the same CSV file. Furthermore, in the future, I may want to handle numbers that have a thousands separator. In the US, we use the comma, e.g. 1 million = 1,000,000. Having , and . as possible decimal separators would be an issue.
  • Since the decimal is now just one character, it should be a case in the main switch statement
• I do not want two data_type() methods. I understand you copied the implementation because you didn't want to cause breaking changes. However, data_type() is an internal method, and there is no contract for internal methods. I believe it is reasonable to add a third parameter to specify the decimal character (optional and defaulting to .)
  • In the future, I may use a struct with more options as the third parameter, but we can cross that bridge when we get there
• Rewriting CSVField::try_parse_decimal() to take the above two points into consideration

[wilfz]

Hello Vincent. Thanks for your kind reply.

• I agree on the possible need for thousands separators. For us in Germany 1 million is 1.000.000, but when working in Switzerland I encountered 1'000'000. So, in other parts of the world, there may be even more weird punctuation for thousands and decimal separators. Thus, a struct with more options as the third parameter is a good idea for the future. Luckily, so far I did not encounter any thousands separators in csv or similar files.
• Certainly, it is not very common to have different decimal separators in the same file. Only, sometimes the programmer does not know in advance what kind of format to expect. That's why I like your idea of format.delimiter({ '\t', ',', ... }) and letting the CSV guesser try out which delimiter to use. But after all; I can also do the decimal conversion with more than one possible decimal symbol within my application.
• The only reason I did not change the data_type() method was not to affect performance by another parameter. If you think it will not matter, I will change my pull request according to your comments.

[wilfz]

I reworked try_parse_decimal() and added the optional parameter const char decimalsymbol = '.' to the internal function data_type().

[vincentlaucsb]

Wrote some unit tests on my end and your changes are passing them just fine. Thanks again for your contribution.