View Issue Details
| ID | Project | Category | View Status | Date Submitted | Last Update |
|---|---|---|---|---|---|
| 0004491 | JEDI VCL | 00 JVCL Components | public | 2008-10-02 14:02 | 2008-10-26 05:40 |
| Reporter | ivobauer | Assigned To | AHUser | ||
| Priority | normal | Severity | minor | Reproducibility | always |
| Status | resolved | Resolution | fixed | ||
| Product Version | Daily / GIT | ||||
| Target Version | Fixed in Version | 3.36 | |||
| Summary | 0004491: TJvCsvDataSet has issues with floating point values | ||||
| Description | TJvCsvDataSet has some issues with exporting/importing floating point values: (1) TJvCsvDataSet uses decimal separator from the system to convert floating point values into their string representation. If the decimal separator taken from the system is the same as a CSV field separator (given by TJvCsvDataSet.Separator property), floating point values are not enclosed with quotes during the export, rendering the generated CSV file unusable. (2) During the import from CSV file, TJvCsvDataSet uses StrToFloatUS on floating point fields in the CSV file to perform conversion between the string representation and the floating point value. In other words, TJvCsvDataSet's import feature is limited to CSV files that use either a dot as the decimal separator or value that is the same as system decimal separator. While not a bug, it severely limits the import capability of otherwise great TJvCsvDataSet. (3)During the export to CSV file, TJvCsvDataSet deliberately uses the system decimal separator to convert floating point values into their string representation. While not a bug, it severely limits the export capability of otherwise great TJvCsvDataSet. | ||||
| Steps To Reproduce | (1) Set Decimal separator in the Windows Local Settings control panel to a comma (','). (2) Create a new project in Delphi, drop a TButton on the main form and wire the following code to its OnClick event: procedure TMainForm.Button1Click(Sender: TObject); function GetFullCsvFilePath(const AFileName: string): string; begin Result := IncludeTrailingPathDelimiter(ExtractFilePath(ParamStr(0))) + AFileName; end; var LDataSet: TJvCsvDataSet; begin LDataSet := TJvCsvDataSet.Create(nil); try LDataSet.CsvFieldDef := 'TIME:%,PRESSURE:&'; LDataSet.FileName := GetFullCsvFilePath('MyCsvTestFile.csv'); LDataSet.LoadsFromFile := False; LDataSet.Open; try LDataSet.EmptyTable; LDataSet.AppendRecord([ 5, 10.01578978]); LDataSet.AppendRecord([10, 10.33778987]); LDataSet.AppendRecord([15, 10.63823532]); LDataSet.AppendRecord([20, 10.85023452]); LDataSet.AppendRecord([25, 11.11343532]); finally LDataSet.Close; end; finally LDataSet.Free; end; end; (3) Compile and run the program. Here is how the contents of exported CSV file look: TIME,PRESSURE 5,10,01578978 10,10,33778987 15,10,63823532 20,10,85023452 25,11,11343532 | ||||
| Additional Information | I suggest the following to be done: (1) For every conversion of floating point values (like import from and export to a CSV file), TJvCsvDataSet should *deliberately* use the decimal separator value taken from a newly introduced read-only property "DecimalSeparator" (see below). (2) The actual value of DecimalSeparator property depends on the value of the newly introduced boolean property "UseSystemDecimalSeparator". If True, the actual value of decimal separator is to be taken from the system settings. If False, the actual value is taken from the newly introduced property "CustomDecimalSeparator". (3) I don't know if the capability is in there already, but TJvCsvDataSet should be able to import CSV files that have floating point values enclosed with quotes. (4) TJvCSvDataSet should be definitely able to detect when the value of its DecimalSeparator property equals to the value of its Separator property and enclose floating point values with quotes during the export to CSV file in such cases. Here is some pseudo-code that demonstrates points (1) and (2) in this additional information section: type TJvCustomCsvDataSet = class(TDataSet) //... protected //... FCustomDecimalSeparator: AnsiChar; FUseSystemDecimalSeparator: Boolean; function GetDecimalSeparator: AnsiChar; property CustomDecimalSeparator: AnsiChar read FCustomDecimalSeparator write FCustomDecimalSeparator default '.'; property DecimalSeparator: Char read GetDecimalSeparator; property UseSystemDecimalSeparator: Boolean read FUseSystemDecimalSeparator write FUseSystemDecimalSeparator default True; //... public //... constructor Create(AOwner: TComponent); override; //... end; constructor TJvCustomCsvDataSet.Create(AOwner: TComponent); begin //... FCustomDecimalSeparator := '.'; FUseSystemDecimalSeparator := True; //... end; function TJvCustomCsvDataSet.GetDecimalSeparator: AnsiChar; begin if UseSystemDecimalSeparator then Result := SysUtils.DecimalSeparator else Result := CustomDecimalSeparator; end; | ||||
| Tags | No tags attached. | ||||
|
|
I'd rather see something like below, to better mimic what other components are doing. type TJvCustomCsvDataSet = class(TDataSet) //... protected //... FDecimalSeparator: AnsiChar; FUseSystemDecimalSeparator: Boolean; function GetDecimalSeparator: AnsiChar; procedure SetDecimalSeparator(const Value: AnsiChar); function IsDecimalSeparatorStored: Boolean; property DecimalSeparator: Char read GetDecimalSeparator write SetDecimalSeparator stored IsDecimalSeparatorStored; property UseSystemDecimalSeparator: Boolean read FUseSystemDecimalSeparator write FUseSystemDecimalSeparator default True; //... public //... constructor Create(AOwner: TComponent); override; //... end; constructor TJvCustomCsvDataSet.Create(AOwner: TComponent); begin //... FDecimalSeparator := '.'; FUseSystemDecimalSeparator := True; //... end; function TJvCustomCsvDataSet.GetDecimalSeparator: AnsiChar; begin if UseSystemDecimalSeparator then Result := SysUtils.DecimalSeparator else Result := FDecimalSeparator; end; procedure TJvCustomCsvDataSet.SetDecimalSeparator(const Value: AnsiChar); begin if FDecimalSeparator <> Value then begin FDecimalSeparator := Value; UseSystemDecimalSeparator := False; end; end; function TJvCustomCsvDataSet.IsDecimalSeparatorStored: Boolean; begin Result := not UseSystemDecimalSeparator; end; |
|
|
I am working on solving this. I understand the problem, and I agree with the person logging the original bug that the usefulness of the component is effectively limited by this current behaviour. In addition, now that I think of it, the software is too fragile in its current state. I believe that even if the regional settings say that the floating point decimal character is ',', that when encountering a file that contains '1.2', which is the normal CSV float format encountered in programs written by English-speaking programmers, the CSVDataset should parse the floating point number correctly. It is not too hard to make the program handle "1,2" in quotes, and "1.2" in quotes, and nobody (thank goodness!) thus far seems to be using decimal points to separate their CSV files. Whew. Looking deeply into the routines in JvCsvData.pas, and the "kludges" we have in place in JvJCLUtils, I believe that routines with functionality like FloatToStrUS can not be used anymore, I will implement an alternative solution to the issue that caused someone to write FloatToStrUS and other related units. This means that once complete, we should remove FloatToStrUS as these "USA-kludge" routines will be orphaned. (They are never called except from JvCsvDataSet). |
|
|
Olivier: You're right about replacing a default character property value by IsDecimalSeparatorStored method, this is the standard approach for character and string properties. However your implementation stores the property value even if it's the same as the default value set by the constructor, which is not necessary. Therefore I'd suggest the following implementation instead: function TJvCustomCsvDataSet.IsDecimalSeparatorStored: Boolean; begin Result := not UseSystemDecimalSeparator and (DecimalSeparator <> '.'); end; |
|
|
Warren: I'd have to disagree with your assertion "file that contains '1.2', which is the normal CSV float format encountered in programs written by English-speaking programmers". (1) Do you happen to have a link to some standard document that governs the usage of decimal separator in CSV files? I couldn't find one, unfortunately. (2) I'm aware of the fact that the English is your native language, but IMO it really does not matter what language the programmer speaks. What matters is a client/customer that this programmer is developing their software for. The key here is the *specification* fo requirements for the software that the programer obtains from their client/customer. And those clients/customers can be from all over the world. (3) I suggest to use the value of DecimalSeparator property *solely* during CSV import when parsing the string representation of floating point values. The users of our applications know the best what is the origin of CSV files they obtained and what kind of decimal separator (and FTR field separator) those CSV files use. Please do not assume anything in your component as it is going to be used in many different scenarios. This means that you can get rid of the FloatToStrUS routine and you don't even need to search for alternative for the purposes of your component. (4) I don't know whether or not it was obvious from my initial post, but I believe that the value of DecimalSeparator property should also be used *solely* during CSV export when converting floating point values into their string representation. Warren, please do not take it as personal offense. I'm just trying to demonstrate my points. |
|
|
No personal offense taken. I stand by my original comments. The CSV standard is used by the vast majority of the programming world, and the vast majority of those users and uses assume a decimal place. There is no standard document, and even if there was, it would be meaningless. To use a german word, 99% of CSV is "gestalt", or to go to latin for a bit, it's "ad-hoc". Thus, there is no documentation for my assertions. The component is being changed from being decimal-as-period centric to being neutral. None of your later comments helped much with that. Please don't take offense. :-) W |
|
|
No personal offense taken. :-) Just wanted to let you know that I really appreciate your time, effort and willingness to make this component culture-neutral and thus usable outside of your world of decimal point separator. Please do not take it as personal offense. Now I'm just worrying about personal offense infinite recursion problems. :-P |
|
|
Fixed checked in to repository. Revision 11946, on October 7, 2008, 10 AM EST. |
|
|
Thank you Warren. I promise to test out the new changes ASAP, but it won't be within the next 2 days because I'm just leaving my town. |
|
|
Warren, I have just finished my tests and your component works exactly as advertised. You did a great work! Thank you so much for these modifications. I think this issue can be marked as resolved now. |
| Date Modified | Username | Field | Change |
|---|---|---|---|
| 2008-10-02 14:02 | ivobauer | New Issue | |
| 2008-10-03 00:16 | obones | Note Added: 0014737 | |
| 2008-10-03 12:56 | wpostma416 | Note Added: 0014739 | |
| 2008-10-03 16:07 | ivobauer | Note Added: 0014742 | |
| 2008-10-03 16:42 | ivobauer | Note Added: 0014743 | |
| 2008-10-06 00:00 | obones | Status | new => confirmed |
| 2008-10-07 05:42 | wpostma416 | Note Added: 0014774 | |
| 2008-10-07 06:45 | ivobauer | Note Added: 0014776 | |
| 2008-10-07 07:12 | wpostma416 | Note Added: 0014777 | |
| 2008-10-07 08:01 | ivobauer | Note Added: 0014779 | |
| 2008-10-09 14:32 | ivobauer | Note Added: 0014798 | |
| 2008-10-26 05:39 | AHUser | Status | confirmed => resolved |
| 2008-10-26 05:39 | AHUser | Fixed in Version | => Daily / SVN |
| 2008-10-26 05:39 | AHUser | Resolution | open => fixed |
| 2008-10-26 05:39 | AHUser | Assigned To | => AHUser |
