[geos-devel] Interface consistency

[strk]

On Tue, Jun 15, 2004 at 10:15:31AM -0400, Frank Warmerdam wrote:
> strk wrote:
> >Before releasing next GEOS I'd like to fix interface inconsistencies.
> >Namely some geometry constructors take ownership over passed
> >components:
> >
> >	o Polygon::Polygon(LinearRing *shell, vector<Geoemtry *> *holes)
> >		will not copy shell and holes, and destroy it at
> >		polygon destruction time.
> >
> >	o GeometryCollection::GeometryCollection(const vector<Geometry 
> >	*>*geoms)
> >		will copy vector, but not geoms, which it will destroy
> >		at GeometryCollection destruction time.
> >
> >	o MultiGEOM::MultiGEOM(const vector<GEOM *>*geoms)
> >		see GeoemtryCollection constructor
> >
> >Other geometry constructors all copy their arguments. I think this needs
> >to be fixed to "all-constructors-copy-arguments". This will break existing
> >applications, but if we make the change now I don't think there will be
> >so much applications involved.
> 
> Strk,
> 
> I ran up against a similar issue in GDAL and ended up having two forms of
> some of the "attach" methods.  For instance, I have an addRing() method on
> the OGRPolygon as well as an addRingDirectly().  The "directly" method takes
> ownership of the passed geometry and can save alot of clone/delete overhead
> in many common situations.
> 
> I gather that the only way to attach the subgeometries to a GEOS geometry
> is during the constructor, is that right?  That is, the geometries are
> essentially immutable after creation?   This makes it harder to have 
> explicit
> means to create attach the geometry with different rules via different named
> methods.

There is an apply_rw method by which geometry modification is possible.

> 
> I would suggest adding a "bool takeOwnership" argument to the constructors
> which is defaulted to false. This allows code that wants to avoid the
> clone/delete overhead to explicitly control ownership but returns the 
> default
> to a consistent approach.

This is what I was thinking about, unfortunately this would not be
transparent... 

> 
> I would add that behavioural changes like this are an awful source of bugs.
> All code written against GEOS so far will need careful review to see if this
> behavioural change affects them.  The sooner a change such as this is made,
> the better.

I know... anyway I'm sure this *really* needs to be done.

> 
> PS. My integration of GEOS into OGR is going very well.

Good to know :)

--strk;