[geos-commits] r2925 - in trunk: . include/geos/operation/polygonize src/operation/polygonize

[svn_geos at osgeo.org]

Author: strk
Date: 2010-02-22 17:17:13 -0500 (Mon, 22 Feb 2010)
New Revision: 2925

Modified:
   trunk/NEWS
   trunk/include/geos/operation/polygonize/Polygonizer.h
   trunk/src/operation/polygonize/Polygonizer.cpp
Log:
Drop heap allocation of invalidRingLines vector

Modified: trunk/NEWS
===================================================================
--- trunk/NEWS	2010-02-22 22:06:33 UTC (rev 2924)
+++ trunk/NEWS	2010-02-22 22:17:13 UTC (rev 2925)
@@ -8,8 +8,10 @@
   - CAPI: GEOSGeom_extractUniquePoints_r
   - CAPI: GEOSGetGeometryN support for single geometries
 - C++ API changes:
-  - Polygonizer::getCutEdges returns a const reference 
-  - Polygonizer::getDangles returns a const reference 
+  - Polygonizer::getCutEdges returns by const ref
+  - Polygonizer::getDangles returns by const ref
+  - Polygonizer::getInvalidRingLines returns by const ref and retains
+    ownership of vector elements
 
 Changes in 3.2.0 
 

Modified: trunk/include/geos/operation/polygonize/Polygonizer.h
===================================================================
--- trunk/include/geos/operation/polygonize/Polygonizer.h	2010-02-22 22:06:33 UTC (rev 2924)
+++ trunk/include/geos/operation/polygonize/Polygonizer.h	2010-02-22 22:17:13 UTC (rev 2925)
@@ -95,10 +95,9 @@
 	 */
 	void polygonize();
 
-	/// @todo : take all args by ref
-	void findValidRings(std::vector<EdgeRing*>& edgeRingList,
-			std::vector<EdgeRing*> *validEdgeRingList,
-			std::vector<geom::LineString*> *invalidRingList);
+	void findValidRings(const std::vector<EdgeRing*>& edgeRingList,
+			std::vector<EdgeRing*>& validEdgeRingList,
+			std::vector<geom::LineString*>& invalidRingList);
 
 	void findShellsAndHoles(const std::vector<EdgeRing*>& edgeRingList);
 
@@ -115,7 +114,7 @@
 	// initialize with empty collections, in case nothing is computed
 	std::vector<const geom::LineString*> dangles;
 	std::vector<const geom::LineString*> cutEdges;
-	std::vector<geom::LineString*> *invalidRingLines;
+	std::vector<geom::LineString*> invalidRingLines;
 
 	std::vector<EdgeRing*> holeList;
 	std::vector<EdgeRing*> shellList;
@@ -203,15 +202,11 @@
 	 * Get the list of lines forming invalid rings found during
 	 * polygonization.
 	 *
-	 * @return a collection of LineStrings which form invalid rings
-	 * Ownership of vector is tranferred to caller.
-	 * On second call will return NULL (unless polygonize is called again).
-	 * Elements of vector will need be released by caller (or will be
-	 * released by this class if nobody ever called this method).
+	 * @return a (possibly empty) collection of pointers to
+	 *         the input LineStrings which form invalid rings
 	 *
-	 * TODO: always retain ownership of vector and elements, return by const ref !
 	 */
-	std::vector<geom::LineString*>* getInvalidRingLines();
+	const std::vector<geom::LineString*>& getInvalidRingLines();
 
 // This seems to be needed by    GCC 2.95.4
 friend class Polygonizer::LineStringAdder;

Modified: trunk/src/operation/polygonize/Polygonizer.cpp
===================================================================
--- trunk/src/operation/polygonize/Polygonizer.cpp	2010-02-22 22:06:33 UTC (rev 2924)
+++ trunk/src/operation/polygonize/Polygonizer.cpp	2010-02-22 22:17:13 UTC (rev 2925)
@@ -64,7 +64,7 @@
 	graph(NULL),
 	dangles(),
 	cutEdges(),
-	invalidRingLines(NULL),
+	invalidRingLines(),
 	holeList(),
 	shellList(),
 	polyList(NULL)
@@ -76,12 +76,9 @@
 	delete lineStringAdder;
 	delete graph;
 
-	if ( invalidRingLines )
-	{
-		for (unsigned int i=0, n=invalidRingLines->size(); i<n; ++i)
-			delete (*invalidRingLines)[i];
-		delete invalidRingLines;
-	}
+	for (unsigned int i=0, n=invalidRingLines.size(); i<n; ++i)
+		delete invalidRingLines[i];
+
 	if ( polyList )
 	{
 		for (unsigned int i=0, n=polyList->size(); i<n; ++i)
@@ -197,17 +194,12 @@
 	return cutEdges;
 }
 
-/*
- * Get the list of lines forming invalid rings found during polygonization.
- * @return a collection of the input {@LineStrings} which form invalid rings
- */
-vector<LineString*>*
+/* public */
+const vector<LineString*>&
 Polygonizer::getInvalidRingLines()
 {
 	polygonize();
-	vector<LineString*> *ret = invalidRingLines;
-	invalidRingLines = NULL;
-	return ret;
+	return invalidRingLines;
 }
 
 /* public */
@@ -232,8 +224,8 @@
 	cerr<<"Polygonizer::polygonize(): "<<edgeRingList.size()<<" edgeRings in graph"<<endl;
 #endif
 	vector<EdgeRing*> validEdgeRingList;
-	invalidRingLines=new vector<LineString*>();
-	findValidRings(edgeRingList, &validEdgeRingList, invalidRingLines);
+	invalidRingLines.clear(); /* what if it was populated already ? we should clean ! */
+	findValidRings(edgeRingList, validEdgeRingList, invalidRingLines);
 #if GEOS_DEBUG
 	cerr<<"                           "<<validEdgeRingList.size()<<" valid"<<endl;
 	cerr<<"                           "<<invalidRingLines.size()<<" invalid"<<endl;
@@ -256,9 +248,9 @@
 
 /* private */
 void
-Polygonizer::findValidRings(vector<EdgeRing*>& edgeRingList,
-	vector<EdgeRing*> *validEdgeRingList,
-	vector<LineString*> *invalidRingList)
+Polygonizer::findValidRings(const vector<EdgeRing*>& edgeRingList,
+	vector<EdgeRing*>& validEdgeRingList,
+	vector<LineString*>& invalidRingList)
 {
 	typedef vector<EdgeRing*> EdgeRingList;
 	
@@ -266,10 +258,14 @@
 	{
 		EdgeRing *er = edgeRingList[i];
 		if (er->isValid())
-			validEdgeRingList->push_back(er);
+		{
+			validEdgeRingList.push_back(er);
+		}
 		else
 		{
-			invalidRingList->push_back(er->getLineString());
+			// NOTE: polygonize::EdgeRing::getLineString
+			// returned LineString ownership is transferred.
+			invalidRingList.push_back(er->getLineString());
 		}
 	}
 }