Skip to content

Commit

Permalink
Add Element::FindElement as an alternative to Element::GetElement (#620)
Browse files Browse the repository at this point in the history 
`Element::GetElement` may create a child element if it doesn't exist.
This makes it a non-const function. Instead of changing this behavior, a
new function `Element::FindElement` is added that returns a nullptr if
the child element is not found instead of creating a new element.


Signed-off-by: Addisu Z. Taddese <addisu@openrobotics.org>
  • Loading branch information
@azeey
azeey committed Jul 9, 2021
1 parent 18f908b commit 8fca2f1
Show file tree
Hide file tree
Showing 4 changed files with 141 additions and 17 deletions.
7 changes: 7 additions & 0 deletions Migration.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,13 @@ forward programmatically.
This document aims to contain similar information to those files
but with improved human-readability..

## libsdformat 9.3 to 9.5

### Additions

1. **sdf/Element.hh**
+ sdf::ElementPtr FindElement() const

## libsdformat 9.3 to 9.4

### Modifications
Expand Down
29 changes: 28 additions & 1 deletion include/sdf/Element.hh
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,10 @@ namespace sdf
/// \brief Shared pointer to an SDF Element
typedef std::shared_ptr<Element> ElementPtr;

/// \def ElementConstPtr
/// \brief Shared pointer to a const SDF Element
typedef std::shared_ptr<const Element> ElementConstPtr;

/// \def ElementWeakPtr
/// \brief Weak pointer to an SDF Element
typedef std::weak_ptr<Element> ElementWeakPtr;
Expand Down Expand Up @@ -359,14 +363,37 @@ namespace sdf
/// \brief Return a pointer to the child element with the provided name.
///
/// A new child element, with the provided name, is added to this element
/// if there is no existing child element.
/// if there is no existing child element. If this is not desired see \ref
/// FindElement
/// \remarks If there are multiple elements with the given tag, it returns
/// the first one.
/// \param[in] _name Name of the child element to retreive.
/// \return Pointer to the existing child element, or a new child
/// element if an existing child element did not exist.
public: ElementPtr GetElement(const std::string &_name);

/// \brief Return a pointer to the child element with the provided name.
///
/// Unlike \ref GetElement, this does not create a new child element if it
/// fails to find an existing element.
/// \remarks If there are multiple elements with the given tag, it returns
/// the first one.
/// \param[in] _name Name of the child element to retreive.
/// \return Pointer to the existing child element, or nullptr
/// if the child element was not found.
public: ElementPtr FindElement(const std::string &_name);

/// \brief Return a pointer to the child element with the provided name.
///
/// Unlike \ref GetElement, this does not create a new child element if it
/// fails to find an existing element.
/// \remarks If there are multiple elements with the given tag, it returns
/// the first one.
/// \param[in] _name Name of the child element to retreive.
/// \return Pointer to the existing child element, or nullptr
/// if the child element was not found.
public: ElementConstPtr FindElement(const std::string &_name) const;

/// \brief Add a named element.
/// \param[in] _name the name of the element to add.
/// \return A pointer to the newly created Element object.
Expand Down
12 changes: 12 additions & 0 deletions src/Element.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -827,6 +827,18 @@ ElementPtr Element::GetElement(const std::string &_name)
return result;
}

/////////////////////////////////////////////////
ElementPtr Element::FindElement(const std::string &_name)
{
return this->GetElementImpl(_name);
}

/////////////////////////////////////////////////
ElementConstPtr Element::FindElement(const std::string &_name) const
{
return this->GetElementImpl(_name);
}

/////////////////////////////////////////////////
void Element::InsertElement(ElementPtr _elem)
{
Expand Down
110 changes: 94 additions & 16 deletions src/Element_TEST.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -825,6 +825,25 @@ TEST(Element, GetNextElementMultiple)
ASSERT_EQ(child2->GetNextElement(""), nullptr);
}

/////////////////////////////////////////////////
/// Helper function to add child elements without having to create descriptions
sdf::ElementPtr addChildElement(sdf::ElementPtr _parent,
const std::string &_elementName,
const bool _addNameAttribute,
const std::string &_childName)
{
sdf::ElementPtr child = std::make_shared<sdf::Element>();
child->SetParent(_parent);
child->SetName(_elementName);
_parent->InsertElement(child);

if (_addNameAttribute)
{
child->AddAttribute("name", "string", _childName, false, "description");
}
return child;
}

/////////////////////////////////////////////////
TEST(Element, CountNamedElements)
{
Expand All @@ -838,22 +857,6 @@ TEST(Element, CountNamedElements)
EXPECT_TRUE(parent->HasUniqueChildNames());
EXPECT_TRUE(parent->HasUniqueChildNames("child"));

auto addChildElement = [](
sdf::ElementPtr _parent,
const std::string &_elementName,
const bool _addNameAttribute,
const std::string &_childName)
{
sdf::ElementPtr child = std::make_shared<sdf::Element>();
child->SetParent(_parent);
child->SetName(_elementName);
_parent->InsertElement(child);

if (_addNameAttribute)
{
child->AddAttribute("name", "string", _childName, false, "description");
}
};

// The following calls should make the following child elements:
// <child name="child1"/>
Expand Down Expand Up @@ -914,6 +917,81 @@ TEST(Element, CountNamedElements)
EXPECT_EQ(allMap.at("child3"), 1u);
}

TEST(Element, FindElement)
{
// <root>
// <elem_A>
// <child_elem_A />
// </elem_A>
//
// <elem_B>
// <child_elem_B name="first_child"/>
// <child_elem_B/>
// </elem_B>
// </root>
sdf::ElementPtr root = std::make_shared<sdf::Element>();
root->SetName("root");

// Create elements
{
auto elemA = addChildElement(root, "elem_A", false, "");
addChildElement(elemA, "child_elem_A", false, "");

auto elemB = addChildElement(root, "elem_B", false, "");
auto firstChildElemB =
addChildElement(elemB, "child_elem_B", true, "first_child");

addChildElement(elemB, "child_elem_B", false, "");
}

{
auto elemA = root->FindElement("elem_A");
ASSERT_NE(nullptr, elemA);
EXPECT_NE(nullptr, elemA->FindElement("child_elem_A"));
EXPECT_EQ(nullptr, elemA->FindElement("non_existent_elem"));

auto elemB = root->FindElement("elem_B");
ASSERT_NE(nullptr, elemB);
// This should find the first "child_elem_B" element, which has the name
// attribute
auto childElemB = elemB->FindElement("child_elem_B");
ASSERT_TRUE(childElemB->HasAttribute("name"));
EXPECT_EQ("first_child", childElemB->GetAttribute("name")->GetAsString());
}

// Check that it works with const pointers
{
sdf::ElementConstPtr rootConst = root;
sdf::ElementConstPtr elemA = rootConst->FindElement("elem_A");
ASSERT_NE(nullptr, elemA);
EXPECT_NE(nullptr, elemA->FindElement("child_elem_A"));
EXPECT_EQ(nullptr, elemA->FindElement("non_existent_elem"));

sdf::ElementConstPtr elemB = root->FindElement("elem_B");
ASSERT_NE(nullptr, elemB);
// This should find the first "child_elem_B" element, which has the name
// attribute
sdf::ElementConstPtr childElemB = elemB->FindElement("child_elem_B");
ASSERT_TRUE(childElemB->HasAttribute("name"));
EXPECT_EQ("first_child", childElemB->GetAttribute("name")->GetAsString());
}
{
sdf::ElementConstPtr rootConst = root;
sdf::ElementConstPtr elemA = rootConst->FindElement("elem_A");
ASSERT_NE(nullptr, elemA);
EXPECT_NE(nullptr, elemA->FindElement("child_elem_A"));
EXPECT_EQ(nullptr, elemA->FindElement("non_existent_elem"));

sdf::ElementConstPtr elemB = root->FindElement("elem_B");
ASSERT_NE(nullptr, elemB);
// This should find the first "child_elem_B" element, which has the name
// attribute
sdf::ElementConstPtr childElemB = elemB->FindElement("child_elem_B");
ASSERT_TRUE(childElemB->HasAttribute("name"));
EXPECT_EQ("first_child", childElemB->GetAttribute("name")->GetAsString());
}
}

/////////////////////////////////////////////////
/// Main
int main(int argc, char **argv)
Expand Down

0 comments on commit 8fca2f1

Please sign in to comment.