return boost::optional for second and penultimate points of SPCurve

(bzr r6616)

3 files changed, 42 insertions, 33 deletions

diff --git a/src/display/curve-test.h b/src/display/curve-test.h
index a45f01afd..56b49ff4c 100644
--- a/src/display/curve-test.h
+++ b/src/display/curve-test.h
@@ -215,32 +215,32 @@ public:
 
     void testSecondPoint()
     {
-        TS_ASSERT_EQUALS(SPCurve(Geom::PathVector(1, path1)).second_point() , Geom::Point(1,0));
-        TS_ASSERT_EQUALS(SPCurve(Geom::PathVector(1, path2)).second_point() , Geom::Point(3,0));
-        TS_ASSERT_EQUALS(SPCurve(Geom::PathVector(1, path3)).second_point() , Geom::Point(5,1));
-        TS_ASSERT_EQUALS(SPCurve(Geom::PathVector(1, path4)).second_point() , Geom::Point(3,5));
+        TS_ASSERT_EQUALS( *(SPCurve(Geom::PathVector(1, path1)).second_point()) , Geom::Point(1,0));
+        TS_ASSERT_EQUALS( *(SPCurve(Geom::PathVector(1, path2)).second_point()) , Geom::Point(3,0));
+        TS_ASSERT_EQUALS( *(SPCurve(Geom::PathVector(1, path3)).second_point()) , Geom::Point(5,1));
+        TS_ASSERT_EQUALS( *(SPCurve(Geom::PathVector(1, path4)).second_point()) , Geom::Point(3,5));
         Geom::PathVector pv;
         pv.push_back(path1);
         pv.push_back(path2);
         pv.push_back(path3);
-        TS_ASSERT_EQUALS(SPCurve(pv).second_point() , Geom::Point(1,0));
+        TS_ASSERT_EQUALS( *(SPCurve(pv).second_point()) , Geom::Point(1,0));
         pv.insert(pv.begin(), path4);
-        TS_ASSERT_EQUALS(SPCurve(pv).second_point() , Geom::Point(0,0));
+        TS_ASSERT_EQUALS( SPCurve(pv).second_point() == false, true );
     }
 
     void testPenultimatePoint()
     {
-        TS_ASSERT_EQUALS(SPCurve(Geom::PathVector(1, path1)).penultimate_point() , Geom::Point(1,1));
-        TS_ASSERT_EQUALS(SPCurve(Geom::PathVector(1, path2)).penultimate_point() , Geom::Point(3,0));
-        TS_ASSERT_EQUALS(SPCurve(Geom::PathVector(1, path3)).penultimate_point() , Geom::Point(6,4));
-        TS_ASSERT_EQUALS(SPCurve(Geom::PathVector(1, path4)).penultimate_point() , Geom::Point(3,5));
+        TS_ASSERT_EQUALS( *(SPCurve(Geom::PathVector(1, path1)).penultimate_point()) , Geom::Point(1,1));
+        TS_ASSERT_EQUALS( *(SPCurve(Geom::PathVector(1, path2)).penultimate_point()) , Geom::Point(3,0));
+        TS_ASSERT_EQUALS( *(SPCurve(Geom::PathVector(1, path3)).penultimate_point()) , Geom::Point(6,4));
+        TS_ASSERT_EQUALS( *(SPCurve(Geom::PathVector(1, path4)).penultimate_point()) , Geom::Point(3,5));
         Geom::PathVector pv;
         pv.push_back(path1);
         pv.push_back(path2);
         pv.push_back(path3);
-        TS_ASSERT_EQUALS(SPCurve(pv).penultimate_point() , Geom::Point(6,4));
+        TS_ASSERT_EQUALS( *(SPCurve(pv).penultimate_point()) , Geom::Point(6,4));
         pv.push_back(path4);
-        TS_ASSERT_EQUALS(SPCurve(pv).penultimate_point() , Geom::Point(8,4));
+        TS_ASSERT_EQUALS( *(SPCurve(pv).penultimate_point()) , Geom::Point(8,4));
     }
 
     // TODO: Rest of the methods
diff --git a/src/display/curve.cpp b/src/display/curve.cpp
index d142b6eac..9f6c34981 100644
--- a/src/display/curve.cpp
+++ b/src/display/curve.cpp
@@ -373,48 +373,55 @@ SPCurve::first_point() const
 
 /**
  * Return the second point of first subpath or _movePos if curve too short.
- * If the pathvector is empty, this returns (0,0). If the first path is only a moveto, this method
+ * If the pathvector is empty, this returns nothing. If the first path is only a moveto, this method
  * returns the first point of the second path, if it exists. If there is no 2nd path, it returns the
  * first point of the first path.
- *
- * FIXME: for empty paths shouldn't this return (NR_HUGE,NR_HUGE)
  */
-Geom::Point
+boost::optional<Geom::Point>
 SPCurve::second_point() const
 {
-    if (is_empty()) {
-        return Geom::Point(0,0);
-    }
-    else if (_pathv.front().empty()) {
-        // first path is only a moveto
-        // check if there is second path
-        if (_pathv.size() > 1) {
-            return _pathv[1].initialPoint();
+    boost::optional<Geom::Point> retval;
+    if (!is_empty()) {
+        if (_pathv.front().empty()) {
+            // first path is only a moveto
+            // check if there is second path
+            if (_pathv.size() > 1) {
+                retval = _pathv[1].initialPoint();
+            } else {
+                retval = _pathv[0].initialPoint();
+            }
         } else {
-            return _pathv[0].initialPoint();
+            retval = _pathv.front()[0].finalPoint();
         }
     }
-    else
-        return _pathv.front()[0].finalPoint();
+
+    return retval;
 }
 
 /**
  * TODO: fix comment: Return the second-last point of last subpath or _movePos if curve too short.
  */
-Geom::Point
+boost::optional<Geom::Point>
 SPCurve::penultimate_point() const
 {
-    Geom::Curve const& back = _pathv.back().back_default();
-    return back.initialPoint();
+    boost::optional<Geom::Point> retval;
+    if (!is_empty()) {
+        Geom::Curve const& back = _pathv.back().back_default();
+        retval = back.initialPoint();
+    }
+
+    return retval;
 }
 
 /**
  * Return last point of last subpath or (0,0).  TODO: shouldn't this be (NR_HUGE, NR_HUGE) to be able to tell it apart from normal (0,0) ?
  * If the last path is only a moveto, then return that point.
  */
-Geom::Point
+boost::optional<Geom::Point>
 SPCurve::last_point() const
 {
+    boost::optional<Geom::Point> retval;
+
     if (is_empty())
         return Geom::Point(0, 0);
 
diff --git a/src/display/curve.h b/src/display/curve.h
index c3659de07..8bb3fb360 100644
--- a/src/display/curve.h
+++ b/src/display/curve.h
@@ -20,6 +20,8 @@
 
 #include <2geom/forward.h>
 
+#include <boost/optional.hpp>
+
 class SPCurve {
 public:
     /* Constructors */
@@ -48,8 +50,8 @@ public:
     Geom::Path const * first_path() const;
     Geom::Point first_point() const;
     Geom::Point last_point() const;
-    Geom::Point second_point() const;
-    Geom::Point penultimate_point() const;
+    boost::optional<Geom::Point> second_point() const;
+    boost::optional<Geom::Point> penultimate_point() const;
 
     void reset();