[fdo-internals] RE: RFC 52 -- Convenience C++ API wrapper for FDO

[Traian Stanev]

Hi Kosta,

Replies are inlined below.

Traian


> -----Original Message-----
> From: Konstantin Baumann
> Sent: Monday, August 09, 2010 4:18 AM
> To: FDO Internals Mail List
> Cc: Traian Stanev
> Subject: RE: RFC 52 -- Convenience C++ API wrapper for FDO
> 
> Hi Traian,
> 
> I like that very much!!
> 
> Some minor comments (but note, I haven't looked into your header files
> yet, just based on the given samples):
> 
> * We had a very similar interface in LDX, but using some more smart
> pointers, that you elegantly removed by your proposal.

There is a single smart pointer (DbHandle) in this proposal, so it's not completely  eliminated, just customized. This handle can also be wrapped in a regular tr1 smart pointer if one needs to hold it for uncertain amount of time.

> 
> * I would propose to use a namespace for the wrapper/library.

I am using the "fdo" namespace. The naming is way too generic otherwise. :)

> 
> * Some method names are not consistent:
> 	- DbHandle::Count()	suggested: DbHanlde::GetTableCount()
> 	- Table::GetRowCount()	suggested: keep it as it is
> 	- RowDef::ColumnCount()	suggested: RowDef::GetColumnCount()

Agree.

> 
> * I would have expected an argument for Table::At(); I would suggest to
> use Table::At(FID id) for random access queries (do you want to support
> them, too?) and Table::Current() for the sequential case.

There is already an At(FID), except I called it Query(FID), since that's what it does in the underlying FDO. I did not want to call it At(FID) since it would imply it's faster than it really is. In addition, I'd like to keep At(integer) for the "scrollable reader" implementation.

> 
> * the samples are missing the returning the connection to the
> connection pool

They are not. :) 
The DbHandle object automatically releases the underlying database back to the pool.

> 
> * discussion about the "get all fields": there is currently an OGR RFC
> discussing this issue right now
> (http://trac.osgeo.org/gdal/wiki/rfc29_desired_fields); something like
> the proposed solution could probably also be easily incorporated into
> your proposal, right?

OK, I'll take a look. 
My immediate ideas for this were to simply define custom Tables with less columns in their definition, since a Table in the wrapper really corresponds more to a select statement, not an actual table (actually it's both, but that's details).

> 
> * is there a need for "bulk inserts"? deletion of features? "bulk
> deletions"?

This should already be happening implicitly for SQLite and SDF data sources, since the lifetime of the Insert command in the wrapper is very long, and that's how bulk stuff is controlled in those providers. Explicit bulk options can of course be added easily by adding Begin/EndTransaction API to the Table class.

> 
> * the samples do not show the definition of a new schema from scratch
> (not by copying an existing one, which is already covered) for creating
> a new database.

The examples don't, but the attached source code does show it, since it creates schemas from OGR and FDO schemas. However, I don't have a user-friendly design for this yet, so it's kind of ugly to code and that's why I didn't show it.

> 
> Overall I like that very much, and I guess that interface would be way
> easier to learn and scare a lot less people than the existing one!
> 
> Kosta
> 
> > -----Original Message-----
> > From: fdo-internals-bounces at lists.osgeo.org [mailto:fdo-internals-
> > bounces at lists.osgeo.org] On Behalf Of Traian Stanev
> > Sent: Monday, August 09, 2010 01:15
> > To: 'FDO Internals Mail List'
> > Subject: [fdo-internals] RFC 52 -- Convenience C++ API wrapper for
> FDO
> >
> > Hello all,
> >
> > As you know, I have recently complained about FDO API complexity [1].
> > Complaining is nice and all, but it's better done in C/C++ form, so I
> have
> > posted a proposal for API simplification as FDO RFC 52. The proposal
> > includes a working draft implementation.
> >
> > If interested, have a look, I'd be interested in comments and
> suggestions
> > for improvement of the design and implementation.
> >
> > http://trac.osgeo.org/fdo/wiki/FDORfc52
> >
> > Thanks,
> > Traian
> >
> >
> > [1] http://osgeo-org.1803224.n2.nabble.com/resignation-from-FDO-PSC-
> > td5302376.html#a5302376
> >
> > _______________________________________________
> > fdo-internals mailing list
> > fdo-internals at lists.osgeo.org
> > http://lists.osgeo.org/mailman/listinfo/fdo-internals