[Kst] kst2 datasource API comments
nicolas.brisset at free.fr
nicolas.brisset at free.fr
Sat Dec 12 02:04:52 CET 2009
as I have seen quite a lot of changes in the datasource API and I understand that feedback is appreciated. 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)!
So, here are the points that I noted (some point may be excessively naive, I'm just doing a brain dump here based on a review of datasource.h and the ascii datasource):
- I really like the approach with 4 types of "primitives" (scalars, strings, vectors, matrices) and the possibility to have metadata. That's something at least the netcdf and cdf datasources will be able to benefit from, as well as ASCII for units. But there are things lacking: 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? Another thing that confuses me with matrices are (x,y)frames and samples, how is that supposed to be handled?
- 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)
- 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...
- having field scalars (or strings) stored in 2 independent lists sounds dangerous, why not use a QHash<QString, double> or similar to store them?
- 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. 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 :-)
Well, I realize this does not make for a very entertaining read :-( Sorry for that, but hopefully it gives a couple of interesting hints nonetheless.
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 :-)
More information about the Kst