[Kst] kst2 datasource API comments

[Peter Kümmel]

Barth Netterfield wrote:
> On Friday 19 March 2010 10:09:08  Peter Kümmel wrote:
>> Attached a patch which shows a solution:
>> We use the same templated interface (SourceInterface) for all Primitives.
>> (The template is needed to pass/get custom parameters. See Vector::Param)
>>
>> This interface must be implemented by the concrete data sources. Primitives
>> not supported simply return a dummy impl (not shown in th patch, see
>> AsciiSource)
>>
>> This way DataSource becomes very simple: all the special functions for all
>> the primitives reduces to one function which returns a interface, eg
>> SourceInterface<Vector>. The runtime overhead is minimal.
>>
>> Using the interface shows DataVector: instead of
>>     rc = _file->readField(_v + _numSamples, _field, new_f0, (new_nf -
>> NF)/Skip, Skip, &lastRead); we have
>>     rc = _file->vector().read(_field, info );
>> and the construction of the parameter object
>>    Vector::Param info = {_v + _numSamples,new_f0, (new_nf - NF)/Skip, Skip,
>> &lastRead};
>>
>> Adding more metainfo to all primitives is simply extending the SourceData
>> template, or we use meta Info type.
>>
>> To read primitive-special values we could do it the same way like when
>> calling read(), we have to add in the primitve type a small structure/clas
>> which will be used by the template, eg with 'T::Info info(const QString&
>> field)'.
>>
>> What do you think? Should I start to port such a design?

Seems the patch wasn't posted to the list, so here the essential part:

template<class T>
struct SourceInterface {

  // read one element
  virtual int read(const QString& name, const typename T::Param&) = 0;

  // named elements
  virtual QStringList list() const = 0;
  virtual bool listIsComplete() const = 0;
  virtual bool isValid(const QString& field) const = 0;

  //frames
  virtual int samplesPerFrame(const QString& field) = 0;
  virtual int frameCount(const QString& field = QString::null) const = 0;

  // meta scalars
  virtual QStringList scalars(const QString& field) = 0;
  virtual int readScalars(QList<double> &v, const QString& field, bool init) = 0;

  // meta strings
  virtual QStringList strings(const QString& field) = 0;
  virtual int readStrings(QStringList &v, const QString& field, bool init) = 0;
};



class KST_EXPORT DataSource : public Object
{
...
    /************************************************************/
    /* Same interface for all supported Primitives              */
    /************************************************************/

    SourceInterface<Vector>& vector() {Q_ASSERT(interf_vector); return *interf_vector; }
    SourceInterface<Scalar>& scalar() {Q_ASSERT(interf_scalar); return *interf_scalar; }
    SourceInterface<String>& string() {Q_ASSERT(interf_string); return *interf_string; }
    SourceInterface<Matrix>& matrix() {Q_ASSERT(interf_matrix); return *interf_matrix; }

   protected:
      SourceInterface<Vector>* interf_vector;
      SourceInterface<Scalar>* interf_scalar;
      SourceInterface<String>* interf_string;
      SourceInterface<Matrix>* interf_matrix;
...
};

> Seems good.  Definitely more C++y.
> 
> This automatically means that datasources can add meta-data to scalars and 
> strings.... right?

Yes, and the old API could be replaced by, as discussed before:
QMap<QString, double> metaScalars() and
QMap<QString, QString> metaStrings()

or should we use the primitive type
QMap<QString, Scalar> metaScalars()
QMap<QString, String> metaStrings()

because when we maybe later wanna add vectors, we need vector objects:
QMap<QString, Vector>
And it would not make much sens to add a new type only for the meta data.


> I like the default dummy interfaces for un-needed primitives.
> 
> For each primitive, there are a lot of 'optional' methods.  For examples, for 
> vectors - the only ones required are read() and frameCount().  The rest just 
> add additional features that not all data sources need support, though they 
> need to exist and return a default value (empty list, 0, etc).  Will default 
> dummies be provided for these in the base interface class?
> 
> I think frameCount only has meaning for vectors.  Will it be part of the 
> template base class (SourceInterface)?

At this point the idea becomes a bit complicated and ugly. I don't wanna pollute
the interface with several primitive-specific functions which other primitive
doesn't support because this breaks the idea of a comman interface.
The cleanest way to solve this is to add two function which sets or returns
the optional data:

class Vector
{
...
  struct Optional {
    int frameCount;
  ...
  };
};


template<class T>
struct SourceInterface {
...
  T::Optional optional() const;
  void setOptional(const T::Optional&);
};

But this is maybe to slow because all optional data is set/read for each call.
Also calling the optional functions is a bit laborious because you always have
to fill/read the Optional structure.

Anyway, I would first use this solution and see how it works and maybe
later on replace it by a more dynamic solution (eg Qt property system
or using QVariant as parameter)


>> +  interf_vector = new AsciiSourceImpl;
>> +  interf_matrix = new DummyMatrixSourceImpl;
> 
> do you mean, imagining that asciisource can provide vectors and strings, but 
> not matrixs or scalars:
> 
> +  interf_vector = new AsciiSourceVectorImpl;
> +  interf_string = new AsciiSourceStringImpl;
> +  interf_matrix = new DummyMatrixSourceImpl;
> +  interf_scalar = new DummyMatrixSourceImpl;

Yes, and the dummy implementations could be already set by DataSource,
asciisource only overwrites its supported types.

(Thinking more about this solution it became clear it's the delegation pattern:
http://en.wikipedia.org/wiki/Delegation_pattern
Where we have, in principle, the possibility do implement the plugins without
inheritance, but I think we don't need it.)

> 
> 
> So: to implement a new data source you implement the datasource, which handles 
> the file opening and (maybe) validation, and then an interface for each of the 
> primitives that you want the data source to be able to support.  Correct?

Yes. Having a interface for each type should make the code very clear.
And the header of the new data source becomes very simple because all the code
could live within the source file:

struct AsciiSourceVector : public SourceInterface<Vector>
{
  // read vector
};

struct AsciiSourceScalar : public SourceInterface<Scalar>
{
  // read scalars, mybe used as units
};


 AsciiSource::AsciiSource()
{
 ...
  interf_scalar = new AsciiSourceScalar;
  interf_vector = new AsciiSourceVector;
}


> 
> If I understand it, I think I like it.

OK, when there are no objection from others (Nicolas?) then I will
do the move. Not identical to above (I think it is better to define
SourceInterface within DataSource and to find a other name; using getters
and setters; ...)

Peter