[geos-devel] Fwd: Prepared Geometry

[Hartmut Kaiser]

Martin, 

> I have to disagree with Hartmut's complaint.  The SegmentString class
> is
> intended to be an adapter class, which wraps a data structure
> (Coordinate sequence) owned by some client to allow it to be processed
> by the noding logic.  Under this pattern, the SegmentString class
> definitely should  *not* be freeing the memory for the wrapped sequence
> - that is the responsibility of the client.

Well, that's a decision of the people around the GEOS project, not mine. I
just fixed some nasty memleaks which wouldn't have been there if some more
coherent memory (and encapsulation) strategy had been used. I'm sure there
was a reason for that. But please don't jump on me, I'm only saying that I
would have done it differently.

> I can't comment on "writing C++ like Java", but obviously C++ requires
> more knowledge of the usage patterns of memory resources than Java
> does.  That's why Java is easier to write  8^)

Come on, this is your personal opinion. And please, there is no need to
start a religious war here. 
I was intentionally very cautious while writing my remark in the first
place.

Regards Hartmut

> 
> Paul Ramsey wrote:
> > Another quarter heard from. I have applied this "ugly" patch
> > nonetheless, since it removes my leak in PostGIS.
> >
> > P
> >
> >
> > ---------- Forwarded message ----------
> > From: Hartmut Kaiser <hartmut.kaiser at gmail.com>
> > Date: Thu, Oct 9, 2008 at 6:55 AM
> > Subject: RE: Prepared Geometry
> > To: Paul Ramsey <pramsey at cleverelephant.ca>
> >
> >
> > Paul,
> >
> >
> >> Could you try big against small1 and small2?
> >>
> >
> > Patch attached. But frankly, I don't like the solution (even if it
> adopted
> > the way it's currently handled all over the place in GEOS).
> >
> > The way the SegmentString class is written and used requires the user
> of the
> > an instance of SegmentString to take care of the deletion of the
> embedded
> > point array, for instance:
> >
> >    for ( size_t i = 0, ni = lineSegStr.size(); i < ni; i++ )
> >    {
> >        delete lineSegStr[ i ]->getCoordinates();    // <-- here
> >        delete lineSegStr[ i ];
> >    }
> >
> > Which is just wrong, because this could be done in a better way if
> > SegmentString was taking care of this in the destructor. I tried to
> fix GEOS
> > (and the tests) in this direction, but apparently the buffers stored
> in the
> > SegmentString are shared in between different higher level structures
> in
> > addition, which makes it impossible (at least currently for me) to
> fix that
> > without introducing some kind of smart pointers (or investing a
> really large
> > amount of time, which I don't have) ...
> >
> > For this reason I fixed this memory leak the straightforward (and
> ugly) way.
> > I'm sure there are more similar bugs to recover, though :-P
> >
> > Regards Hartmut
> >
> > PS: As impressive GEOS is it convinces me once more that it's not a
> good
> > idea to write C++ in a way similar to Java, especially if no special
> means
> > of memory management (i.e. smart ptrs) are introduced.
> >
> >
> >
> >> Thanks!
> >>
> >> Paul
> >>
> >> On Wed, Oct 8, 2008 at 1:54 PM, Hartmut Kaiser
> >> <hartmut.kaiser at gmail.com> wrote:
> >>
> >>> Paul,
> >>>
> >>>
> >>>> The reason I ask is because while contains and covers appear leak
> >>>> free, intersects seems to be leaking. I'm pretty sure it's not on
> >>>>
> >> the
> >>
> >>>> PostGIS side (I think).
> >>>>
> >>>> P.
> >>>>
> >>>> On Wed, Oct 8, 2008 at 12:25 PM, Paul Ramsey
> >>>> <pramsey at cleverelephant.ca> wrote:
> >>>>
> >>>>> Hartmut,
> >>>>>
> >>>>> When you tested for leaks, did you test prepared contains,
> >>>>>
> >>>> intersects,
> >>>>
> >>>>> covers, coversproperly? Or just one of them?
> >>>>>
> >>> I was using the test program as attached to the ticket (which AFAIR
> >>> calls all of the topological attribute functions). This test
> program
> >>> was leak free for me. But I tried the geometries as attached to the
> >>> ticket only either, so your leaks might show themselves with
> certain
> >>>
> >> geometries only.
> >>
> >>> Regards Hartmut
> >>>
> >>>
> >>>
> >>>
> >>>
> >>> -------------------------------------------------------------------
> -----
> >>>
> >>> _______________________________________________
> >>> geos-devel mailing list
> >>> geos-devel at lists.osgeo.org
> >>> http://lists.osgeo.org/mailman/listinfo/geos-devel
> 
> --
> Martin Davis
> Senior Technical Architect
> Refractions Research, Inc.
> (250) 383-3022
> 
> _______________________________________________
> geos-devel mailing list
> geos-devel at lists.osgeo.org
> http://lists.osgeo.org/mailman/listinfo/geos-devel