[Kst] kst2 datasource API comments

[Barth Netterfield]

Thanks.
On Friday 11 December 2009 20:04:52 nicolas.brisset at free.fr wrote:
> Hi,
> I must say that it would be nice to provide source compatibility for 
> a while, because a porting effort like
>  the switch from 1.x to 2.x datasources requires is quite a pain (even
>  though I admit it was required)!

Yup. 

> limiting per-source metadata to scalars and strings sounds
>  OK, but why not provide the same per-matrix metadata (as is done for
>  vectors)? And per-scalar strings? 

It would be trivial to add all the special cases to the API.  (That would be 
16 calls.  matrixVectors, scalarMatrixes, stringStrings, etc...) It would be 
greater to do it with template functions or some such, but once you get down 
into the datasource itself, it will probably always be special cases.  Hmmm...  
Not sure the best approach.

>  Another thing that confuses me with
>  matrices are (x,y)frames and samples, how is that supposed to be handled?

1 sample per frame always?  So frames == samples.  Can't think off hand of a 
different case.

>  - I am not sure it is appropriate to have write functions in the
>  data*source* (it would be better to define a data*sink* for that)

I agree. 

>  - there
>  are lots of small functions which I'm wondering about. It would at least
>  be good to have somewhere a clear list of the minimum to implement when
>  creating a new datasource, to avoid overwhelming potential new
>  contributors. Examples include init() vs initPlugins(), cleanupForExit()
>  vs the destructor, findOrLoadSource() vs loadSource(), validSource() vs
>  isValid(), configWidgetForSource() vs configWidgetForPlugin() vs
>  configWidget(), save() vs saveSource(), etc... 

This all needs cleanup.  I also don't know what is going on there.  However, 
there is a demo data source which can be filled in pretty easily.  However, 
most of the things you mention don't need to be re-implemented in the data 
source.  Documentation in the header should be improved.

>  - having field scalars (or
>  strings) stored in 2 independent lists sounds dangerous, why not use a
>  QHash<QString, double> or similar to store them? 

Speed.  readFieldStrings gets called on every vector update, so we want to do 
the very bare minimum.  In the case we have now, only the data, not the names 
need to be changed at each update.  But: I do agree it is more dangerous in 
principle.  

>  - I find the ASCII datasource pretty complex. It's hard not to get lost
>  between the AsciiSource (derived from Kst::DataSource), Ascii::Config 
>  and AsciiPlugin (derived from Kst::DataSourcePluginInterface) classes. 

Agreed.  I don't know what is going on with some of this either.

>  In the .cpp you get
>  the expected ConfigWidgetAscii, which is no more in ascii.h than
>  Ascii::Config. And I'm not even mentioning the fact that Ascii::Config has
>  read but no write and that it mixes working with QSettings and the XML
>  .kst file. Or count the number of times there is the string "Comment
>  Delimiters": 6, qnd it's only counting the management of settings! Phew! I
>  don't remember it being so complex when I improved it a bit a few years
>  ago. I wonder if I wouldn't just have given up if it had been as complex
>  as today! And by the way, I think that Peter forgot to save properly the
>  "use dot" property in the XML when he introduced it, which is probably a
>  sign that it needs some refactoring. But that's actually more the ASCII
>  datasource itself than the Kst::DataSource class in itself, which I
>  guess is rather good news :-)

I'm wondering if it might not want a pretty complete refactor/rewrite. 

> Now I understand better why the trolls spend so much time on
>  API reviews, and also why a face-to-face meeting with a whiteboard is more
>  efficient :-)

That would be very cool.  In the mean time, perhaps an interface document 
would be in order :-)

cbn