View Issue Details
| ID | Project | Category | View Status | Date Submitted | Last Update |
|---|---|---|---|---|---|
| 0004675 | JEDI VCL | 00 JVCL Components | public | 2009-01-26 03:59 | 2009-07-03 11:31 |
| Reporter | dcabale | Assigned To | obones | ||
| Priority | normal | Severity | minor | Reproducibility | always |
| Status | resolved | Resolution | fixed | ||
| Product Version | 3.34 | ||||
| Target Version | Fixed in Version | 3.38 | |||
| Summary | 0004675: [TJvCustomCsvDataSet] procs. GetFileName + Flush: must not raise exception when FTableName = '' | ||||
| Description | function TJvCustomCsvDataSet.GetFileName is the setter function for FOpenFileName and it currently raises an exception if FTableName = ''. GetFileName function is called with only one procedure, InternalOpen. Later in this InternalOpen proc, FTableName is tested together with FLoadsFromFile: "if (FTableName = '') and FLoadsFromFile then ..". Thus GetFileName's FTableName exception overrides InternalOpen's FTableName condition and all subsequent conditions where FTableName is tested More generally, every lines of code that raises exceptions on FTableName = '' and that override subsequent conditions that test FTableName = '', must be deleted. These to_be_removed exceptions are: - in function TJvCustomCsvDataSet.GetFileName: "Assert(Length(FTableName) <> 0, RsEInvalidTableName);". This overrides InternalOpen's testing - in procedure TJvCustomCsvDataSet.Flush: "if FTableName = '' then raise EJvCsvDataSetError.CreateRes(@RsETableNameNotSet);". This overrides later testing in same procedure. | ||||
| Tags | No tags attached. | ||||
|
|
Why is it better to silently ignore the fact that FTableName is empty? |
|
|
That's actually a good point to discuss, and I will answer from analytical point of view, and from a technical point of view: ** from analytical point of view ** FTableName property is usefull when you want to load from a file at the time you open the TJvCsvDataSet. But there are times when you want to populate the TJvCsvDataSet manually (or copy from another dataset) and then save it to a .csv file. ** from technical point of view ** Please look at procedure TJvCustomCsvDataSet.InternalOpen; 1. InternalOpen is the only procedure where GetFileName is called 2. InternalOpen raises an exception, which I agree with, if 2 conditions are true: FTableName is empty *together with* FLoadsFromFile is true Thus one may want to have a FTableName set to '' and a FLoadsFromFile set to false. Is my view clearer? |
|
|
Here is another way to get this topic fixed. My goal is to find a way of "not being forced to load the TJvCsvDataSet from a *.csv file when opening it". The solution is simply to "set FLoadsFromFile to false". And that is maybe the way this component should be used. Ok, let's say this component should be used like this, that is: 1. being forced to set a FTableName <> '', otherwise an exception is raised 2. if don't want to load from a *.csv, then setting FLoadsFromFile to false 3. if want to load from a *.csv, then setting FLoadsFromFile to true Assuming this, the TJvCsvDataSet's current code could be simplified: 1. why the redundant exception handling about FTableName in procedure TJvCustomCsvDataSet.InternalOpen? [code] FOpenFileName := GetFileName; ... if (FTableName = '') and FLoadsFromFile then .. [/code] 2. why the redundant condition handling about FTableName in procedure TJvCustomCsvDataSet.Flush? [code] if FTableName = '' then raise EJvCsvDataSetError.CreateRes(@RsETableNameNotSet); if FFileDirty and FSavesChanges and (Length(FTableName) > 0) then [/code] Now you have 2 propositions: the one described in my 1st post and the one above |
|
|
A fix has been added to SVN. The assert is still there but should no longer trigger when not using file to load/save data |
| Date Modified | Username | Field | Change |
|---|---|---|---|
| 2009-01-26 03:59 | dcabale | New Issue | |
| 2009-02-02 01:27 | obones | Status | new => acknowledged |
| 2009-04-29 11:46 | obones | Note Added: 0015452 | |
| 2009-04-29 11:46 | obones | Status | acknowledged => feedback |
| 2009-04-29 17:08 | dcabale | Note Added: 0015495 | |
| 2009-05-02 21:37 | dcabale | Note Added: 0015508 | |
| 2009-07-03 11:31 | obones | Note Added: 0015745 | |
| 2009-07-03 11:31 | obones | Status | feedback => resolved |
| 2009-07-03 11:31 | obones | Fixed in Version | => Daily / SVN |
| 2009-07-03 11:31 | obones | Resolution | open => fixed |
| 2009-07-03 11:31 | obones | Assigned To | => obones |
