[gdal-dev] Draft GDAL/OGR class hierarchy for GDAL 2.0

[Even Rouault]

Selon Etienne Tourigny <etourigny.dev at gmail.com>:

> I also (respectfully) think that there are too many classes (mostly the
> abstract classes and interfaces) which make it a bit hard to understand.
>
> I like Vincent's second suggestion to have 3 base Dataset classes
> (GDALRasterDataset, GDALVectorDataset, GDALHybridDataset) which inherit
> from GDALDataset, and an enum (vector,raster,hybrid) to easily query which
> type the Dataset is (or which type its driver supports). You then subclass
> the relevant Dataset class and it *should* work auto-magically for rasters.
>  For example, a given Dataset which subclasses GDALDataset would be
> migrated as  a subclass of GDALRasterDataset, which would imply that it
> does not have a vector dataset. If the implementation changes to allow
> vector, then you would change its parent and implement relevant vector
> methods.
>
> My view on this is KISS with a minimum number of classes...

Yes, the practice shows that the UML brainstorming leads to nowhere (at least
with my current limited knowledge of C++ multi-inheritance)... So actually
instead of creating new classes, I think that eventually there will be less
classes... I'm not even sure about the interest of having subclasses of
GDALDataset to distinguish between raster only, vector only or raster+vector. As
I wrote previously that could be done as a driver metadata instread.

So, for now, I've just "imported" the usefull methods of OGRDataSource into
GDALDataset, and made a few changes here or there so that it works. Well except
ogr_refcount.py, because of a funny discovery was that the ref counting of
GDALDataset and OGRDataSource is (was) not the same. Starts at 1 for GDALDataset
and 0 for OGRDataSource...

The temporary result is in the following branch: 
https://github.com/rouault/gdal-1/commits/unification

OGRDataSource is kept with just that :

class CPL_DLL OGRDataSource : public GDALDataset
{
public:
                        OGRDataSource();

    virtual const char  *GetName() = 0;

    static void         DestroyDataSource( OGRDataSource * );

    OGRSFDriver        *GetOGRDriver() const;
    void                SetOGRDriver( OGRSFDriver *poDriver );
    virtual     const char* GetDriverName();

protected:
    friend class OGRSFDriverRegistrar;

    OGRSFDriver        *m_poOGRDriver;

};

And it could eventually completely disappear (or maybe just remain as a
convenience class if we don't want to touch existing drivers) if I manage to
merge OGRSFDriver into GDALDriver, and get rid of OGRSFDriverRegistrar.

Even

>
> cheers
> Etienne
>
>
>
> On Tue, Mar 25, 2014 at 9:13 AM, Vincent Mora
> <vincent.mora at oslandia.com>wrote:
>
> > On 25/03/2014 11:44, Even Rouault wrote:
> >
> >> Selon Vincent Mora <vincent.mora at oslandia.com>:
> >>
> >>  On 24/03/2014 21:46, Even Rouault wrote:
> >>>
> >>>> Hi,
> >>>>
> >>>> "Release soon, release early", so for people who like UML diagrams
> >>>> (there
> >>>>
> >>> is
> >>>
> >>>> also a prototype of C++ classes for those who don't like UML very much),
> >>>>
> >>> here's
> >>>
> >>>> a blog entry with the outcome of my thoughts for a possible
> >>>> re-organisation
> >>>>
> >>> of
> >>>
> >>>> the GDAL/OGR class hierarchy
> >>>>
> >>>>
> >>>>  http://erouault.blogspot.ca/2014/03/draft-gdalogr-class-
> >> hierarchy-for-gdal.html
> >>
> >>> I don't understand the need for HandleRasterData() and
> >>> HandleVectorData().
> >>>
> >>> Isn't inheriting from GDALEmptyRasterDataset or GDALEmptyVectorDataset
> >>> sufficient to convey the information that the class doesn't handle those
> >>> kind of datasets.
> >>>
> >>> A call to GetLayerCount()/GetRasterCount() from the class user is
> >>> sufficient to say "no vector/raster data here", and a dynamic _cast to
> >>> GDALEmptyVectorDataset/GDALEmptyRasterDataset is even clearer to me, so
> >>> HandleRasterData()/HandleVectorData() would be a third way to tell the
> >>> same thing.
> >>>
> >> Actually, when thinking more about this, this kind of information should
> >> be at
> >> the driver level, and not at the dataset level. The idea is that there
> >> are use
> >> cases where you want to know if a driver can handle only vector, only
> >> raster, or
> >> both. For example if you still want to have separate Open dialog boxes for
> >> raster or vector.
> >>
> >>> Also I'm not sure about the names GDALAbstract* since those are partial
> >>> implementations.
> >>>
> >> I know I'm not very good at naming. Do you have an alternative proposal ?
> >> The
> >> issue is that with my draft we end up with a lot of classes and
> >> interfaces, so
> >> it is not obvious to find a good name to reflect their content.
> >>
> > Partial sound ok to me, but I not so good at naming  ?
> >
> > Considering the number of classes, I gave it some thoughts this morning
> > and, like Dmitry, thought of merging the Abtract (Partial) into the
> > interface before realizing that, even if it works (you can overload de
> > default function in Empty) it's a bit ugly and I ended up preferring your
> > solution.
> >
> > An altenative would be to have a Dataset with both aspects and provide
> > three partial specialisations: one for vector (it will behave like empty
> > raster) one for raster, and one for both. Code duplications could be
> > avoided by implementing protected member functions in the Dataset class and
> > simply calling them in the implementations of partial specialization. This
> > solution avoid the diamond shaped inheritance diagram of hell :)
> >
> >    Even
> >>
> >>
> >>
> > _______________________________________________
> > gdal-dev mailing list
> > gdal-dev at lists.osgeo.org
> > http://lists.osgeo.org/mailman/listinfo/gdal-dev
> >
>