[2302] Fix bugs in Element class

Two bugs in src/lib/cc/data.h:

* Element::find() does not return value because its argument is not a
  reference.

* getValue() member functions does not work for ConstElementPointer
  because getValue() member functions are not specified as const.

Aharen found these bugs.

diff --git a/src/lib/cc/data.cc b/src/lib/cc/data.cc
index 47b1eb1fa4edab9ec4817e220e768241bc34e6df..8d9b87dd05254d1fba5f65bd7a419043b9e8b349 100644 (file)
--- a/src/lib/cc/data.cc
+++ b/src/lib/cc/data.cc
@@ -57,32 +57,32 @@ Element::toWire(std::ostream& ss) const {
 }
 
 bool
-Element::getValue(long int&) {
+Element::getValue(long int&) const {
     return (false);
 }
 
 bool
-Element::getValue(double&) {
+Element::getValue(double&) const {
     return (false);
 }
 
 bool
-Element::getValue(bool&) {
+Element::getValue(bool&) const {
     return (false);
 }
 
 bool
-Element::getValue(std::string&) {
+Element::getValue(std::string&) const {
     return (false);
 }
 
 bool
-Element::getValue(std::vector<ConstElementPtr>&) {
+Element::getValue(std::vector<ConstElementPtr>&) const {
     return (false);
 }
 
 bool
-Element::getValue(std::map<std::string, ConstElementPtr>&) {
+Element::getValue(std::map<std::string, ConstElementPtr>&) const {
     return (false);
 }
 
@@ -167,7 +167,7 @@ Element::find(const std::string&) const {
 }
 
 bool
-Element::find(const std::string&, ConstElementPtr) const {
+Element::find(const std::string&, ConstElementPtr&) const {
     return (false);
 }
 
@@ -812,7 +812,7 @@ MapElement::set(const std::string& key, ConstElementPtr value) {
 }
 
 bool
-MapElement::find(const std::string& id, ConstElementPtr t) const {
+MapElement::find(const std::string& id, ConstElementPtr& t) const {
     try {
         ConstElementPtr p = find(id);
         if (p) {

diff --git a/src/lib/cc/data.h b/src/lib/cc/data.h
index 5c731e6b54112c0201bfad37221b10856c3a326a..ef177ca988739402910e616c1eb29de0ed55c3d3 100644 (file)
--- a/src/lib/cc/data.h
+++ b/src/lib/cc/data.h
@@ -152,12 +152,12 @@ public:
     /// data to the given reference and returning true
     ///
     //@{
-    virtual bool getValue(long int& t);
-    virtual bool getValue(double& t);
-    virtual bool getValue(bool& t);
-    virtual bool getValue(std::string& t);
-    virtual bool getValue(std::vector<ConstElementPtr>& t);
-    virtual bool getValue(std::map<std::string, ConstElementPtr>& t);
+    virtual bool getValue(long int& t) const;
+    virtual bool getValue(double& t) const;
+    virtual bool getValue(bool& t) const;
+    virtual bool getValue(std::string& t) const;
+    virtual bool getValue(std::vector<ConstElementPtr>& t) const;
+    virtual bool getValue(std::map<std::string, ConstElementPtr>& t) const;
     //@}
 
     ///
@@ -253,7 +253,7 @@ public:
     /// \param identifier The identifier of the element to find
     /// \param t Reference to store the resulting ElementPtr, if found.
     /// \return true if the element was found, false if not.
-    virtual bool find(const std::string& identifier, ConstElementPtr t) const;
+    virtual bool find(const std::string& identifier, ConstElementPtr& t) const;
     //@}
 
 
@@ -378,7 +378,7 @@ public:
     IntElement(long int v) : Element(integer), i(v) { }
     long int intValue() const { return (i); }
     using Element::getValue;
-    bool getValue(long int& t) { t = i; return (true); }
+    bool getValue(long int& t) const { t = i; return (true); }
     using Element::setValue;
     bool setValue(const long int v) { i = v; return (true); }
     void toJSON(std::ostream& ss) const;
@@ -392,7 +392,7 @@ public:
     DoubleElement(double v) : Element(real), d(v) {};
     double doubleValue() const { return (d); }
     using Element::getValue;
-    bool getValue(double& t) { t = d; return (true); }
+    bool getValue(double& t) const { t = d; return (true); }
     using Element::setValue;
     bool setValue(const double v) { d = v; return (true); }
     void toJSON(std::ostream& ss) const;
@@ -406,7 +406,7 @@ public:
     BoolElement(const bool v) : Element(boolean), b(v) {};
     bool boolValue() const { return (b); }
     using Element::getValue;
-    bool getValue(bool& t) { t = b; return (true); }
+    bool getValue(bool& t) const { t = b; return (true); }
     using Element::setValue;
     bool setValue(const bool v) { b = v; return (true); }
     void toJSON(std::ostream& ss) const;
@@ -427,7 +427,7 @@ public:
     StringElement(std::string v) : Element(string), s(v) {};
     std::string stringValue() const { return (s); }
     using Element::getValue;
-    bool getValue(std::string& t) { t = s; return (true); }
+    bool getValue(std::string& t) const { t = s; return (true); }
     using Element::setValue;
     bool setValue(const std::string& v) { s = v; return (true); }
     void toJSON(std::ostream& ss) const;
@@ -441,7 +441,7 @@ public:
     ListElement() : Element(list) {}
     const std::vector<ConstElementPtr>& listValue() const { return (l); }
     using Element::getValue;
-    bool getValue(std::vector<ConstElementPtr>& t) {
+    bool getValue(std::vector<ConstElementPtr>& t) const {
         t = l;
         return (true);
     }
@@ -474,7 +474,7 @@ public:
         return (m);
     }
     using Element::getValue;
-    bool getValue(std::map<std::string, ConstElementPtr>& t) {
+    bool getValue(std::map<std::string, ConstElementPtr>& t) const {
         t = m;
         return (true);
     }
@@ -507,7 +507,7 @@ public:
     // returns true if found, or false if not found (either because
     // it doesnt exist or one of the elements in the path is not
     // a MapElement)
-    bool find(const std::string& id, ConstElementPtr t) const;
+    bool find(const std::string& id, ConstElementPtr& t) const;
 
     bool equals(const Element& other) const;
 };

diff --git a/src/lib/cc/tests/data_unittests.cc b/src/lib/cc/tests/data_unittests.cc
index 87d92f64ab0095039e6cd0f4c75023b32c841712..7da91e2eb4e99fb4bcb91af68da8e1b2940b9361 100644 (file)
--- a/src/lib/cc/tests/data_unittests.cc
+++ b/src/lib/cc/tests/data_unittests.cc
@@ -176,6 +176,7 @@ TEST(Element, create_and_value_throws) {
     // this test checks whether elements throw exceptions if the
     // incorrect type is requested
     ElementPtr el;
+    ConstElementPtr cel;
     long int i;
     double d;
     bool b;
@@ -197,6 +198,22 @@ TEST(Element, create_and_value_throws) {
     EXPECT_FALSE(el->getValue(v));
     EXPECT_FALSE(el->getValue(m));
     EXPECT_EQ(i, 1);
+
+    cel = Element::create(1);
+    EXPECT_NO_THROW(cel->intValue());
+    EXPECT_THROW(cel->doubleValue(), TypeError);
+    EXPECT_THROW(cel->boolValue(), TypeError);
+    EXPECT_THROW(cel->stringValue(), TypeError);
+    EXPECT_THROW(cel->listValue(), TypeError);
+    EXPECT_THROW(cel->mapValue(), TypeError);
+    EXPECT_TRUE(cel->getValue(i));
+    EXPECT_FALSE(cel->getValue(d));
+    EXPECT_FALSE(cel->getValue(b));
+    EXPECT_FALSE(cel->getValue(s));
+    EXPECT_FALSE(cel->getValue(v));
+    EXPECT_FALSE(cel->getValue(m));
+    EXPECT_EQ(i, 1);
+
     i = 2;
     EXPECT_TRUE(el->setValue(i));
     EXPECT_EQ(2, el->intValue());
@@ -401,6 +418,7 @@ TEST(Element, MapElement) {
     EXPECT_EQ(el->find("value1/")->stringValue(), "bar");
     
     EXPECT_TRUE(el->find("value1", el2));
+    EXPECT_EQ("bar", el2->stringValue());
     EXPECT_FALSE(el->find("name/error", el2));
 
     // A map element whose (only) element has the maximum length of tag.