[geos-devel] Virtual inheritance: call for test
strk
strk at keybit.net
Fri Feb 4 07:59:53 EST 2011
You may have noticed recent commits focused on removing static casts.
Under the hood this was propedeutic work to introduction of virtual
inheritance, allowing us to map the concept of Puntal, Lineal and
Polygonal JTS interfaces into GEOS.
Once such change is introduced, you'll be able to upcast Puntal, Lineal
and Polygonal to Geometry.
And you may upcast Point and MultiPoint to Puntal,
LineString or MultiLineString to Lineal, Polygon and MultiPolygon to
Polygonal.
A side-effect is that you cannot downcast from Geometry to Point
or LineString or Polygon w/out using dynamic_cast anymore.
You'll get a compiler error if you try (which drove the changes).
You may notice that casting up and down between Geometry and one
of the basic types you'll really get different pointers, which is
the reason why static_cast is forbidden.
GCC kindly complains both with static_cast _and_ with C-like casts,
so the real danger is only when you use reinterpret_cast<> or you
step on a void pointer. In both cases you'll get no warning at compile
time but memory errors at runtime. r3184 is an example of such bad
hiding bug that took a while to find (due to the unfortunate interface
of STR tree making use of void pointers).
I do belive the change is healthy, if not else, for forcing cleanup up
of dangling unsafe casts and pointing out other unsafe interfaces.
Before committing this to SVN trunk I'd like to have some feedback
by anyone having C++ client code [1] to build and test againts it.
Both the GEOS and the PostGIS testsuites run fine with the patch applied.
The patch is in mailbox format, can apply using git-am or patch -p1 from
top-level source dir.
[1] Consider also adding the project on the wiki, if not
already listed: http://trac.osgeo.org/geos/wiki/Applications
--strk;
() Free GIS & Flash consultant/developer
/\ http://strk.keybit.net/services.html
-------------- next part --------------
>From 52ee164ca58cf3811e66bf300e6e65587428f4f5 Mon Sep 17 00:00:00 2001
From: Sandro Santilli <strk at keybit.net>
Date: Fri, 4 Feb 2011 13:36:17 +0100
Subject: [PATCH] Turn Puntal, Lineal and Polygonal into Geometry derivates. This commit introduces virtual inheritance and 3 diamonds.
---
NEWS | 2 ++
include/geos/geom/GeometryCollection.h | 2 +-
include/geos/geom/LineString.h | 6 +-----
include/geos/geom/Lineal.h | 6 +++++-
include/geos/geom/MultiLineString.inl | 1 +
include/geos/geom/MultiPoint.h | 2 +-
include/geos/geom/MultiPolygon.inl | 1 +
include/geos/geom/Point.h | 2 +-
include/geos/geom/Polygon.h | 2 +-
include/geos/geom/Polygonal.h | 6 +++++-
include/geos/geom/Puntal.h | 6 +++++-
src/geom/LinearRing.cpp | 4 +++-
src/geom/MultiLineString.cpp | 10 ++++++----
src/geom/MultiPoint.cpp | 1 +
src/geom/MultiPolygon.cpp | 3 ++-
15 files changed, 36 insertions(+), 18 deletions(-)
diff --git a/NEWS b/NEWS
index abad81a..76110b1 100644
--- a/NEWS
+++ b/NEWS
@@ -16,6 +16,8 @@ Changes in 3.3.0
- CAPI: GEOSRelatePatternMatch
- PHP: new PHP5 bindings based on CAPI
- C++ API changes:
+ - Geometry inheritance chain changed to introduce Puntal, Lineal
+ and Polygonal classes (virtual inheritance introduced)
- Geometry::isWithinDistance method is now const
- Polygonizer::getCutEdges returns by const ref
- Polygonizer::getDangles returns by const ref
diff --git a/include/geos/geom/GeometryCollection.h b/include/geos/geom/GeometryCollection.h
index 9e2a8cc..b16b094 100644
--- a/include/geos/geom/GeometryCollection.h
+++ b/include/geos/geom/GeometryCollection.h
@@ -54,7 +54,7 @@ namespace geom { // geos::geom
* represented by GeometryCollection subclasses MultiPoint,
* MultiLineString, MultiPolygon.
*/
-class GEOS_DLL GeometryCollection : public Geometry {
+class GEOS_DLL GeometryCollection : public virtual Geometry {
public:
friend class GeometryFactory;
diff --git a/include/geos/geom/LineString.h b/include/geos/geom/LineString.h
index 95f6a78..074c0ac 100644
--- a/include/geos/geom/LineString.h
+++ b/include/geos/geom/LineString.h
@@ -68,7 +68,7 @@ namespace geom { // geos::geom
* If these conditions are not met, the constructors throw
* an {@link IllegalArgumentException}
*/
-class GEOS_DLL LineString: public Geometry, public Lineal {
+class GEOS_DLL LineString: public virtual Geometry, public Lineal {
public:
@@ -223,10 +223,6 @@ LineString::clone() const {
} // namespace geos::geom
} // namespace geos
-//#ifdef GEOS_INLINE
-//# include "geos/geom/LineString.inl"
-//#endif
-
#ifdef _MSC_VER
#pragma warning(pop)
#endif
diff --git a/include/geos/geom/Lineal.h b/include/geos/geom/Lineal.h
index 5a63cbc..d639dc0 100644
--- a/include/geos/geom/Lineal.h
+++ b/include/geos/geom/Lineal.h
@@ -27,7 +27,11 @@ namespace geom { // geos::geom
* Identifies {@link Geometry} subclasses which
* are 1-dimensional and with components which are {@link LineString}s.
*/
-class Lineal {};
+class Lineal : public virtual Geometry
+{
+protected:
+ Lineal(): Geometry(0) {}
+};
} // namespace geos::geom
} // namespace geos
diff --git a/include/geos/geom/MultiLineString.inl b/include/geos/geom/MultiLineString.inl
index 884c49c..e3f72cc 100644
--- a/include/geos/geom/MultiLineString.inl
+++ b/include/geos/geom/MultiLineString.inl
@@ -32,6 +32,7 @@ namespace geom { // geos::geom
INLINE
MultiLineString::MultiLineString(const MultiLineString &mp)
:
+ Geometry(mp),
GeometryCollection(mp)
{
}
diff --git a/include/geos/geom/MultiPoint.h b/include/geos/geom/MultiPoint.h
index f6f7fcd..d22110a 100644
--- a/include/geos/geom/MultiPoint.h
+++ b/include/geos/geom/MultiPoint.h
@@ -104,7 +104,7 @@ protected:
*/
MultiPoint(std::vector<Geometry *> *newPoints, const GeometryFactory *newFactory);
- MultiPoint(const MultiPoint &mp): GeometryCollection(mp) {}
+ MultiPoint(const MultiPoint &mp): Geometry(mp), GeometryCollection(mp) {}
const Coordinate* getCoordinateN(int n) const;
};
diff --git a/include/geos/geom/MultiPolygon.inl b/include/geos/geom/MultiPolygon.inl
index 23574d4..e7bdf36 100644
--- a/include/geos/geom/MultiPolygon.inl
+++ b/include/geos/geom/MultiPolygon.inl
@@ -29,6 +29,7 @@ namespace geom { // geos::geom
INLINE
MultiPolygon::MultiPolygon(const MultiPolygon &mp)
:
+ Geometry(mp),
GeometryCollection(mp)
{
}
diff --git a/include/geos/geom/Point.h b/include/geos/geom/Point.h
index 8169b4a..4829f72 100644
--- a/include/geos/geom/Point.h
+++ b/include/geos/geom/Point.h
@@ -65,7 +65,7 @@ namespace geom { // geos::geom
* (i.e does not have an NaN X or Y ordinate)
*
*/
-class GEOS_DLL Point : public Geometry, public Puntal
+class GEOS_DLL Point : public virtual Geometry, public Puntal
{
public:
diff --git a/include/geos/geom/Polygon.h b/include/geos/geom/Polygon.h
index 5466f0a..5367932 100644
--- a/include/geos/geom/Polygon.h
+++ b/include/geos/geom/Polygon.h
@@ -64,7 +64,7 @@ namespace geom { // geos::geom
* Specification for SQL</A> .
*
*/
-class GEOS_DLL Polygon: public Geometry, public Polygonal
+class GEOS_DLL Polygon: public virtual Geometry, public Polygonal
{
public:
diff --git a/include/geos/geom/Polygonal.h b/include/geos/geom/Polygonal.h
index 1af44fc..b2afe2b 100644
--- a/include/geos/geom/Polygonal.h
+++ b/include/geos/geom/Polygonal.h
@@ -27,7 +27,11 @@ namespace geom { // geos::geom
* Identifies {@link Geometry} subclasses which
* are 2-dimensional and with components which are {@link Polygon}s.
*/
-class Polygonal {};
+class Polygonal : public virtual Geometry
+{
+protected:
+ Polygonal(): Geometry(0) {}
+};
} // namespace geos::geom
} // namespace geos
diff --git a/include/geos/geom/Puntal.h b/include/geos/geom/Puntal.h
index 6298fee..0b47a96 100644
--- a/include/geos/geom/Puntal.h
+++ b/include/geos/geom/Puntal.h
@@ -27,7 +27,11 @@ namespace geom { // geos::geom
* Identifies {@link Geometry} subclasses which
* are 0-dimensional and with components which are {@link Point}s.
*/
-class Puntal {};
+class Puntal : public virtual Geometry
+{
+protected:
+ Puntal(): Geometry(0) {}
+};
} // namespace geos::geom
} // namespace geos
diff --git a/src/geom/LinearRing.cpp b/src/geom/LinearRing.cpp
index 1807b34..2baaca0 100644
--- a/src/geom/LinearRing.cpp
+++ b/src/geom/LinearRing.cpp
@@ -35,12 +35,13 @@ namespace geos {
namespace geom { // geos::geom
/*public*/
-LinearRing::LinearRing(const LinearRing &lr): LineString(lr) {}
+LinearRing::LinearRing(const LinearRing &lr): Geometry(lr), LineString(lr) {}
/*public*/
LinearRing::LinearRing(CoordinateSequence* newCoords,
const GeometryFactory *newFactory)
:
+ Geometry(newFactory),
LineString(newCoords, newFactory)
{
validateConstruction();
@@ -50,6 +51,7 @@ LinearRing::LinearRing(CoordinateSequence* newCoords,
LinearRing::LinearRing(CoordinateSequence::AutoPtr newCoords,
const GeometryFactory *newFactory)
:
+ Geometry(newFactory),
LineString(newCoords, newFactory)
{
validateConstruction();
diff --git a/src/geom/MultiLineString.cpp b/src/geom/MultiLineString.cpp
index bdc3306..ceace2a 100644
--- a/src/geom/MultiLineString.cpp
+++ b/src/geom/MultiLineString.cpp
@@ -45,6 +45,7 @@ namespace geom { // geos::geom
MultiLineString::MultiLineString(vector<Geometry *> *newLines,
const GeometryFactory *factory)
:
+ Geometry(factory),
GeometryCollection(newLines,factory)
{
}
@@ -71,8 +72,9 @@ bool MultiLineString::isClosed() const {
if (isEmpty()) {
return false;
}
- for (size_t i = 0; i < geometries->size(); i++) {
- if (!((LineString *)(*geometries)[i])->isClosed()) {
+ for (size_t i = 0, n = geometries->size(); i < n; ++i) {
+ LineString *ls = dynamic_cast<LineString*>((*geometries)[i]);
+ if ( ! ls->isClosed() ) {
return false;
}
}
@@ -112,8 +114,8 @@ MultiLineString::reverse() const
Geometry::NonConstVect *revLines = new Geometry::NonConstVect(nLines);
for (size_t i=0; i<nLines; ++i)
{
- assert(dynamic_cast<LineString*>((*geometries)[i]));
- LineString *iLS = static_cast<LineString*>((*geometries)[i]);
+ LineString *iLS = dynamic_cast<LineString*>((*geometries)[i]);
+ assert(iLS);
(*revLines)[nLines-1-i] = iLS->reverse();
}
return getFactory()->createMultiLineString(revLines);
diff --git a/src/geom/MultiPoint.cpp b/src/geom/MultiPoint.cpp
index 36a17b6..6b6b0be 100644
--- a/src/geom/MultiPoint.cpp
+++ b/src/geom/MultiPoint.cpp
@@ -35,6 +35,7 @@ namespace geom { // geos::geom
/*protected*/
MultiPoint::MultiPoint(vector<Geometry *> *newPoints, const GeometryFactory *factory)
:
+ Geometry(factory),
GeometryCollection(newPoints,factory)
{
}
diff --git a/src/geom/MultiPolygon.cpp b/src/geom/MultiPolygon.cpp
index 0c2e40f..0f0515f 100644
--- a/src/geom/MultiPolygon.cpp
+++ b/src/geom/MultiPolygon.cpp
@@ -41,7 +41,8 @@ namespace geom { // geos::geom
/*protected*/
MultiPolygon::MultiPolygon(vector<Geometry *> *newPolys, const GeometryFactory *factory)
- : GeometryCollection(newPolys,factory)
+ : Geometry(factory),
+ GeometryCollection(newPolys,factory)
{}
MultiPolygon::~MultiPolygon(){}
--
1.7.0.4
More information about the geos-devel
mailing list