Cleanup VoronoiDiagramBuilder interface, add NEWS item (libgeos#627)

Cleanups involved removing all explicit "delete" calls trough
auto_ptr uses, moving some allocations from heap to stack and
reducing object copies.

git-svn-id: http://svn.osgeo.org/geos/trunk@3955 5242fede-7e19-0410-aef8-94bd7d2200fb

NEWS

@@ -2,6 +2,7 @@ Changes in 3.5.0
 2013-MM-DD
 
 - New things:
+  - Voronoi API (#627)
   ...
 
 Changes in 3.4.2

include/geos/triangulate/VoronoiDiagramBuilder.h

@@ -20,6 +20,7 @@
 #define GEOS_TRIANGULATE_VORONOIDIAGRAMBUILDER_H
 
 #include <geos/triangulate/quadedge/QuadEdgeSubdivision.h>
+#include <geos/geom/Envelope.h> // for composition 
 #include <memory>
 #include <iostream>
 
@@ -29,7 +30,6 @@ namespace geos {
 	class CoordinateSequence;
 	class GeometryCollection;
 	class GeometryFactory;
-	class Envelope;
 }
 namespace triangulate { //geos.triangulate
 
@@ -44,13 +44,6 @@ namespace triangulate { //geos.triangulate
  *
  */
 class GEOS_DLL VoronoiDiagramBuilder{
-private:
-	geom::CoordinateSequence* siteCoords;
-	double tolerance;
-	quadedge::QuadEdgeSubdivision* subdiv;
-	geom::Envelope* clipEnv;
-	geom::Envelope* diagramEnv;
-
 public:
 	/** 
 	 * Creates a new Voronoi diagram builder.
@@ -81,9 +74,11 @@ class GEOS_DLL VoronoiDiagramBuilder{
 	 * The diagram will be clipped to the larger
 	 * of this envelope or an envelope surrounding the sites.
 	 * 
-	 * @param clipEnv the clip envelope.
+	 * @param clipEnv the clip envelope; must be kept alive by
+	 *                caller until done with this instance;
+	 *                set to 0 for no clipping.
 	 */
-	void setClipEnvelope(const geom::Envelope& clipEnv);
+	void setClipEnvelope(const geom::Envelope* clipEnv);
 
 	/**
 	 * Sets the snapping tolerance which will be used
@@ -92,14 +87,14 @@ class GEOS_DLL VoronoiDiagramBuilder{
 	 * 
 	 * @param tolerance the tolerance distance to use
 	 */
-	void setTolerance(const double tolerance);
+	void setTolerance(double tolerance);
 
 	/**
 	 * Gets the {@link QuadEdgeSubdivision} which models the computed diagram.
 	 * 
 	 * @return the subdivision containing the triangulation
 	 */
-	quadedge::QuadEdgeSubdivision* getSubdivision();
+	std::auto_ptr<quadedge::QuadEdgeSubdivision> getSubdivision();
 
 	/**
 	 * Gets the faces of the computed diagram as a {@link GeometryCollection} 
@@ -111,6 +106,13 @@ class GEOS_DLL VoronoiDiagramBuilder{
 	std::auto_ptr<geom::GeometryCollection> getDiagram(const geom::GeometryFactory& geomFact);
 
 private:
+
+	std::auto_ptr<geom::CoordinateSequence> siteCoords;
+	double tolerance;
+	std::auto_ptr<quadedge::QuadEdgeSubdivision> subdiv;
+	const geom::Envelope* clipEnv; // externally owned
+	geom::Envelope diagramEnv;
+
 	void create();
 
 	static std::auto_ptr<geom::GeometryCollection> 

src/triangulate/VoronoiDiagramBuilder.cpp

@@ -38,70 +38,61 @@ using namespace geos::geom;
 
 
 VoronoiDiagramBuilder::VoronoiDiagramBuilder() :
-	siteCoords(NULL), tolerance(0.0), subdiv(NULL) , clipEnv(NULL), diagramEnv(NULL)
+	tolerance(0.0), clipEnv(0)
 {
 }
 
 VoronoiDiagramBuilder::~VoronoiDiagramBuilder()
 {
-	if(siteCoords)
-		delete siteCoords;
-	if(subdiv)
-		delete subdiv;
-	if(clipEnv)
-	   	delete clipEnv;
-	if(diagramEnv)
-	   	delete diagramEnv;
 }
 
 void 
 VoronoiDiagramBuilder::setSites(const geom::Geometry& geom)
 {
-	siteCoords = DelaunayTriangulationBuilder::extractUniqueCoordinates(geom);
+	siteCoords.reset( DelaunayTriangulationBuilder::extractUniqueCoordinates(geom) );
 }
 
 void 
 VoronoiDiagramBuilder::setSites(const geom::CoordinateSequence& coords)
 {
-	siteCoords = coords.clone();
+	siteCoords.reset( coords.clone() );
 	DelaunayTriangulationBuilder::unique(*siteCoords);
 }
 
 void 
-VoronoiDiagramBuilder::setClipEnvelope(const geom::Envelope& clipEnv)
+VoronoiDiagramBuilder::setClipEnvelope(const geom::Envelope* nClipEnv)
 {
-	*(this->clipEnv) = clipEnv;
+	clipEnv = nClipEnv;
 }
 
 void 
-VoronoiDiagramBuilder::setTolerance(const double tolerance)
+VoronoiDiagramBuilder::setTolerance(double nTolerance)
 {
-	this->tolerance = tolerance;
+	tolerance = nTolerance;
 }
 
 void 
 VoronoiDiagramBuilder::create()
 {
-	if(subdiv!=NULL)
-		return;
-	geom::Envelope siteEnv = DelaunayTriangulationBuilder::envelope(*siteCoords);
-	diagramEnv = new Envelope();
-	*diagramEnv = siteEnv;
+	if( subdiv.get() ) return;
+
+	diagramEnv = DelaunayTriangulationBuilder::envelope(*siteCoords);
 	//adding buffer around the final envelope
-	double expandBy = fmax(diagramEnv->getWidth() , diagramEnv->getHeight());
-	diagramEnv->expandBy(expandBy);
-	if(clipEnv!=NULL)
-		diagramEnv->expandToInclude(clipEnv);
+	double expandBy = fmax(diagramEnv.getWidth() , diagramEnv.getHeight());
+	diagramEnv.expandBy(expandBy);
+	if(clipEnv)
+		diagramEnv.expandToInclude(clipEnv);
 
-	IncrementalDelaunayTriangulator::VertexList* vertices = DelaunayTriangulationBuilder::toVertices(*siteCoords);
+	std::auto_ptr<IncrementalDelaunayTriangulator::VertexList> vertices (
+    DelaunayTriangulationBuilder::toVertices(*siteCoords)
+  );
 
-	subdiv = new quadedge::QuadEdgeSubdivision(*diagramEnv,tolerance);
-	IncrementalDelaunayTriangulator triangulator(subdiv);
+	subdiv.reset( new quadedge::QuadEdgeSubdivision(diagramEnv,tolerance) );
+	IncrementalDelaunayTriangulator triangulator(subdiv.get());
 	triangulator.insertSites(*vertices);
-	delete vertices;
 }
 
-quadedge::QuadEdgeSubdivision* 
+std::auto_ptr<quadedge::QuadEdgeSubdivision> 
 VoronoiDiagramBuilder::getSubdivision()
 {
 	create();
@@ -113,35 +104,35 @@ VoronoiDiagramBuilder::getDiagram(const geom::GeometryFactory& geomFact)
 {
 	create();
 	std::auto_ptr<geom::GeometryCollection> polys = subdiv->getVoronoiDiagram(geomFact);
-	return clipGeometryCollection(*polys,*diagramEnv);
+	return clipGeometryCollection(*polys,diagramEnv);
 }
 
 std::auto_ptr<geom::GeometryCollection> 
 VoronoiDiagramBuilder::clipGeometryCollection(const geom::GeometryCollection& geom, const geom::Envelope& clipEnv)
 {
-	geom::Geometry* clipPoly = geom.getFactory()->toGeometry(&clipEnv);
+	std::auto_ptr<geom::Geometry> clipPoly ( geom.getFactory()->toGeometry(&clipEnv) );
 	std::auto_ptr< std::vector<Geometry*> >clipped(new std::vector<Geometry*>);
 	for(std::size_t i=0 ; i < geom.getNumGeometries() ; i++)
 	{
-		Geometry* g = (Geometry*)geom.getGeometryN(i);
-		Geometry* result=NULL;
+		const Geometry* g = geom.getGeometryN(i);
+		std::auto_ptr<Geometry> result;
 		// don't clip unless necessary
 		if(clipEnv.contains(g->getEnvelopeInternal()))
 		{
-			result = g->clone();
+			result.reset( g->clone() );
+      // TODO: check if userData is correctly cloned here?
 		}
 		else if(clipEnv.intersects(g->getEnvelopeInternal()))
 		{
-			result = clipPoly->intersection(g);
-			result->setUserData(g->getUserData());
+			result.reset( clipPoly->intersection(g) );
+			result->setUserData(((Geometry*)g)->getUserData()); // TODO: needed ?
 		}
 
-		if(result!=NULL && !result->isEmpty() )
+		if(result.get() && !result->isEmpty() )
 		{
-			clipped->push_back(result);
+			clipped->push_back(result.release());
 		}
 	}
-	delete clipPoly;
 	return std::auto_ptr<GeometryCollection>(geom.getFactory()->createGeometryCollection(clipped.release()));
 }
 

tests/unit/triangulate/VoronoiTest.cpp

@@ -47,10 +47,10 @@ namespace tut
 	void runVoronoi(const char *sitesWkt, const char *expectedWkt , const double tolerance)
 	{
 		WKTReader reader;
-	        WKTWriter writer;
-	        geos::triangulate::VoronoiDiagramBuilder builder;
-		Geometry* sites = reader.read(sitesWkt);
-		Geometry* expected = reader.read(expectedWkt);
+		WKTWriter writer;
+		geos::triangulate::VoronoiDiagramBuilder builder;
+		std::auto_ptr<Geometry> sites ( reader.read(sitesWkt) );
+		std::auto_ptr<Geometry> expected ( reader.read(expectedWkt) );
 		std::auto_ptr<GeometryCollection> results;
 		GeometryFactory geomFact;
 		builder.setSites(*sites);
@@ -62,11 +62,9 @@ namespace tut
 		results->normalize();
 		expected->normalize();
 
-		ensure(results->equalsExact(expected, 1e-7));
+		ensure(results->equalsExact(expected.get(), 1e-7));
 		ensure(results->getCoordinateDimension() == expected->getCoordinateDimension());
 
-		delete sites;
-		delete expected;
 	}
 
 	// Test Cases
@@ -88,15 +86,14 @@ namespace tut
 		v->push_back(c);
 		v->push_back(d);
 
-		geos::geom::CoordinateArraySequence *seq = new CoordinateArraySequence(v.release());
-		builder.setSites(*seq);
+		geos::geom::CoordinateArraySequence seq(v.release());
+		builder.setSites(seq);
 
 		//getting the subdiv()
-		QuadEdgeSubdivision* subdiv = builder.getSubdivision();
+		std::auto_ptr<QuadEdgeSubdivision> subdiv = builder.getSubdivision();
 
 		ensure_equals(subdiv->getTolerance() , 0);
 		ensure_equals(subdiv->getEnvelope().toString(),"Env[-3540:4020,-3436:4050]");
-		delete seq;
 
 	}
 	// 1 - Case with a single point