View Issue Details

IDProjectCategoryView StatusLast Update
0004675JEDI VCL00 JVCL Componentspublic2009-07-03 11:31
ReporterdcabaleAssigned Toobones 
PrioritynormalSeverityminorReproducibilityalways
Status resolvedResolutionfixed 
Product Version3.34 
Target VersionFixed in Version3.38 
Summary0004675: [TJvCustomCsvDataSet] procs. GetFileName + Flush: must not raise exception when FTableName = ''
Descriptionfunction 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.
TagsNo tags attached.

Activities

obones

2009-04-29 11:46

administrator   ~0015452

Why is it better to silently ignore the fact that FTableName is empty?

dcabale

2009-04-29 17:08

reporter   ~0015495

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?

dcabale

2009-05-02 21:37

reporter   ~0015508

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

obones

2009-07-03 11:31

administrator   ~0015745

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

Issue History

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