[geos-commits] r2276 - in trunk: source/operation/distance tests/unit/operation/distance

[svn_geos at osgeo.org]

Author: mloskot
Date: 2009-03-11 08:51:20 -0400 (Wed, 11 Mar 2009)
New Revision: 2276

Modified:
   trunk/source/operation/distance/DistanceOp.cpp
   trunk/tests/unit/operation/distance/DistanceOpTest.cpp
Log:
BUG in DistanceOp:
 * Test case and explanation of existing bug in DistanceOp::closestPoints() recently reported by Aya (Ticket #236).
 * Refactored closestPoints() method to clearly present where is the bug. Again, chain calls are evil! Bless clear code!
 * tests/unit/operation/distance/DistanceOpTest.cpp: see test case test<17>, read FIXME comments. Uncomment closestPoints() to run and reproduce the bug.
 * A dirty fix: if loc0 or loc1 are nullptr, return nullptr CoordinatesSequence from closestPoints().

Modified: trunk/source/operation/distance/DistanceOp.cpp
===================================================================
--- trunk/source/operation/distance/DistanceOp.cpp	2009-03-11 12:18:47 UTC (rev 2275)
+++ trunk/source/operation/distance/DistanceOp.cpp	2009-03-11 12:51:20 UTC (rev 2276)
@@ -109,10 +109,27 @@
 CoordinateSequence*
 DistanceOp::closestPoints()
 {
-	computeMinDistance();
-	CoordinateSequence* closestPts=new CoordinateArraySequence();
-	closestPts->add((*minDistanceLocation)[0]->getCoordinate());
-	closestPts->add((*minDistanceLocation)[1]->getCoordinate());
+    assert(0 != minDistanceLocation);
+    std::vector<GeometryLocation*>& locs = *minDistanceLocation;
+
+    computeMinDistance();
+
+    // FIXME: This code (refactored to make the bug visible) causes
+    //        crash reported in Ticket #236
+    // NOTE: In order to reproduce, uncomment test case test<17>
+    //       in tests/unit/operation/distance/DistanceOpTest.cpp        
+    GeometryLocation* loc0 = locs[0];
+    GeometryLocation* loc1 = locs[1];
+    // Assertion fails, so...
+    assert(0 != loc0 && 0 != loc1);
+    // ...accesss violation is thrown.
+    Coordinate& c0 = loc0->getCoordinate();
+    Coordinate& c1 = loc1->getCoordinate();
+
+    CoordinateSequence* closestPts = new CoordinateArraySequence();
+	closestPts->add(c0);
+	closestPts->add(c1);
+
 	return closestPts;
 }
 

Modified: trunk/tests/unit/operation/distance/DistanceOpTest.cpp
===================================================================
--- trunk/tests/unit/operation/distance/DistanceOpTest.cpp	2009-03-11 12:18:47 UTC (rev 2275)
+++ trunk/tests/unit/operation/distance/DistanceOpTest.cpp	2009-03-11 12:51:20 UTC (rev 2276)
@@ -7,11 +7,12 @@
 // GEOS
 #include <geos/operation/distance/DistanceOp.h>
 #include <geos/platform.h>
+#include <geos/geom/Coordinate.h>
 #include <geos/geom/GeometryFactory.h>
 #include <geos/geom/Geometry.h>
 #include <geos/algorithm/PointLocator.h>
 #include <geos/io/WKTReader.h>
-#include <geos/geom/Coordinate.h>
+#include <geos/geom/CoordinateSequence.h>
 #include <memory>
 #include <vector>
 
@@ -30,14 +31,9 @@
 		typedef geos::geom::Geometry::AutoPtr GeomPtr;
 
 		test_distanceop_data()
-			:
-			gf(),
-			wktreader(&gf)
-		{
-		}
-
+            : gf(), wktreader(&gf)
+		{}
 	};
-	
 
 	typedef test_group<test_distanceop_data> group;
 	typedef group::object object;
@@ -47,7 +43,6 @@
 	//
 	// Test Cases
 	//
-
 	template<>
 	template<>
 	void object::test<1>()
@@ -65,7 +60,8 @@
 
 		// TODO: test closestPoints() and
 		//       closestLocations()
-
+        geos::geom::CoordinateSequence* coords = dist.closestPoints();
+        ensure(0 != coords);
 	}
 
 	template<>
@@ -380,6 +376,28 @@
 
 	}
 
+    // Test for crash reported in Ticket #236:
+    // http://trac.osgeo.org/geos/ticket/236
+	template<>
+	template<>
+	void object::test<17>()
+	{
+		using geos::operation::distance::DistanceOp;
+
+		std::string wkt0("POLYGON((0 0, 0 1, 1 1, 1 0, 0 0))");
+		std::string wkt1("POLYGON((1.25 0.25, 1.25 0.75, 1.75 0.75, 1.75 0.25, 1.25 0.25))");
+
+		GeomPtr g0(wktreader.read(wkt0));
+		GeomPtr g1(wktreader.read(wkt1));
+
+		DistanceOp dist(g0.get(), g1.get());
+		ensure_equals(dist.distance(), 0.25);
+
+        // FIXME: This operation crashes becasue of both GeometryLocation's are NULL
+        //        in DistanceOp::closestPoints()
+        //geos::geom::CoordinateSequence* cs dist.closestPoints();
+	}
+
 	// TODO: finish the tests by adding:
 	// 	LINESTRING - *all*
 	// 	MULTILINESTRING - *all*