From a89b4d216841ba23337f649c5810ea621ad4f106 Mon Sep 17 00:00:00 2001
From: dellaert <dellaert@cc.gatech.edu>
Date: Fri, 28 Nov 2014 17:13:54 +0100
Subject: [PATCH] empty_ is gone

---
 gtsam/base/OptionalJacobian.h             | 15 ++++---
 gtsam/base/tests/testOptionalJacobian.cpp | 50 +++++++++++++++--------
 2 files changed, 40 insertions(+), 25 deletions(-)

diff --git a/gtsam/base/OptionalJacobian.h b/gtsam/base/OptionalJacobian.h
index 9b5615423..91cd1089a 100644
--- a/gtsam/base/OptionalJacobian.h
+++ b/gtsam/base/OptionalJacobian.h
@@ -44,7 +44,6 @@ public:
 
 private:
 
-  bool empty_; ///< flag whether initialized or not
   Eigen::Map<Fixed> map_; /// View on constructor argument, if given
 
   // Trick from http://eigen.tuxfamily.org/dox/group__TutorialMapClass.html
@@ -57,25 +56,25 @@ public:
 
   /// Default constructor acts like boost::none
   OptionalJacobian() :
-      empty_(true), map_(NULL) {
+      map_(NULL) {
   }
 
   /// Constructor that will usurp data of a fixed-size matrix
   OptionalJacobian(Fixed& fixed) :
-      empty_(false), map_(NULL) {
+      map_(NULL) {
     usurp(fixed.data());
   }
 
   /// Constructor that will usurp data of a fixed-size matrix, pointer version
   OptionalJacobian(Fixed* fixedPtr) :
-      empty_(fixedPtr==NULL), map_(NULL) {
+      map_(NULL) {
     if (fixedPtr)
       usurp(fixedPtr->data());
   }
 
   /// Constructor that will resize a dynamic matrix (unless already correct)
   OptionalJacobian(Eigen::MatrixXd& dynamic) :
-      empty_(false), map_(NULL) {
+      map_(NULL) {
     dynamic.resize(Rows, Cols); // no malloc if correct size
     usurp(dynamic.data());
   }
@@ -84,12 +83,12 @@ public:
 
   /// Constructor with boost::none just makes empty
   OptionalJacobian(boost::none_t none) :
-      empty_(true), map_(NULL) {
+      map_(NULL) {
   }
 
   /// Constructor compatible with old-style derivatives
   OptionalJacobian(const boost::optional<Eigen::MatrixXd&> optional) :
-      empty_(!optional), map_(NULL) {
+      map_(NULL) {
     if (optional) {
       optional->resize(Rows, Cols);
       usurp(optional->data());
@@ -100,7 +99,7 @@ public:
 
   /// Return true is allocated, false if default constructor was used
   operator bool() const {
-    return !empty_;
+    return map_.data();
   }
 
   /// De-reference, like boost optional
diff --git a/gtsam/base/tests/testOptionalJacobian.cpp b/gtsam/base/tests/testOptionalJacobian.cpp
index a03757e60..6584c82d1 100644
--- a/gtsam/base/tests/testOptionalJacobian.cpp
+++ b/gtsam/base/tests/testOptionalJacobian.cpp
@@ -24,21 +24,37 @@ using namespace std;
 using namespace gtsam;
 
 //******************************************************************************
-TEST( OptionalJacobian, Constructors )
-{
+TEST( OptionalJacobian, Constructors ) {
   Matrix23 fixed;
+
+  OptionalJacobian<2, 3> H1;
+  EXPECT(!H1);
+
+  OptionalJacobian<2, 3> H2(fixed);
+  EXPECT(H2);
+
+  OptionalJacobian<2, 3> H3(&fixed);
+  EXPECT(H3);
+
   Matrix dynamic;
-  OptionalJacobian<2,3> H1;
-  OptionalJacobian<2,3> H2(fixed);
-  OptionalJacobian<2,3> H3(&fixed);
-  OptionalJacobian<2,3> H4(dynamic);
-  OptionalJacobian<2,3> H5(boost::none);
+  OptionalJacobian<2, 3> H4(dynamic);
+  EXPECT(H4);
+
+  OptionalJacobian<2, 3> H5(boost::none);
+  EXPECT(!H5);
+
   boost::optional<Matrix&> optional(dynamic);
-  OptionalJacobian<2,3> H6(optional);
+  OptionalJacobian<2, 3> H6(optional);
+  EXPECT(H6);
 }
 
 //******************************************************************************
-void test3(OptionalJacobian<2,3> H = OptionalJacobian<2,3>()) {
+void test(OptionalJacobian<2, 3> H = boost::none) {
+  if (H)
+    *H = Matrix23::Zero();
+}
+
+void testPtr(Matrix23* H = NULL) {
   if (H)
     *H = Matrix23::Zero();
 }
@@ -49,35 +65,35 @@ TEST( OptionalJacobian, Ref2) {
   expected = Matrix23::Zero();
 
   // Default argument does nothing
-  test3();
+  test();
 
   // Fixed size, no copy
   Matrix23 fixed1;
   fixed1.setOnes();
-  test3(fixed1);
+  test(fixed1);
   EXPECT(assert_equal(expected,fixed1));
 
   // Fixed size, no copy, pointer style
   Matrix23 fixed2;
   fixed2.setOnes();
-  test3(&fixed2);
+  test(&fixed2);
   EXPECT(assert_equal(expected,fixed2));
 
   // Empty is no longer a sign we don't want a matrix, we want it resized
   Matrix dynamic0;
-  test3(dynamic0);
+  test(dynamic0);
   EXPECT(assert_equal(expected,dynamic0));
 
   // Dynamic wrong size
-  Matrix dynamic1(3,5);
+  Matrix dynamic1(3, 5);
   dynamic1.setOnes();
-  test3(dynamic1);
+  test(dynamic1);
   EXPECT(assert_equal(expected,dynamic0));
 
   // Dynamic right size
-  Matrix dynamic2(2,5);
+  Matrix dynamic2(2, 5);
   dynamic2.setOnes();
-  test3(dynamic2);
+  test(dynamic2);
   EXPECT(assert_equal(dynamic2,dynamic0));
 }