Add Component::extractComponent and Component::removeComponent

CHANGELOG.md

@@ -33,6 +33,7 @@ Scholz et al. (2015). This class encapsulates `SimTK::CableSpan`, the Simbody im
 geometry for path-based OpenSim components (e.g., `Muscle`, `PathSpring`, etc.). This class can be used as a replacement for `GeometryPath`, providing improved computational
 performance and stability in wrapping solutions.
 - Implemented `generateDecorations()` for `Station` to allow visualization in the Simbody visualizer. (#4169)
+- Added `Component::removeComponent` and `Component::extractComponent` methods, which enable removing subcomponents that were previously added via `Component::addComponent` or the `<components>` XML element (#4174).
 
 
 v4.5.2

OpenSim/Common/Component.cpp

@@ -159,6 +159,33 @@ void Component::addComponent(Component* subcomponent)
     extendAddComponent(subcomponent);
 }
 
+std::unique_ptr<Component> Component::extractComponent(Component* subcomponent)
+{
+    auto& componentsProp = updProperty_components();
+
+    // Try to find `subcomponent` in the `components` property.
+    int idx = -1;
+    for (int i = 0; i < componentsProp.size(); ++i) {
+        if (&componentsProp[i] == subcomponent) {
+            idx = i;
+            break;
+        }
+    }
+    if (idx == -1) {
+        return nullptr;  // Not found.
+    }
+
+    // Perform removal
+    std::unique_ptr<Component> rv = componentsProp.extractValueAtIndex(idx);
+    finalizeFromProperties();
+    return rv;
+}
+
+bool Component::removeComponent(Component* subcomponent)
+{
+    return extractComponent(subcomponent) != nullptr;  // `std::unique_ptr<Component>` handles destruction
+}
+
 void Component::prependComponentPathToConnecteePath(
         Component& subcomponent) {
     const std::string compPath = subcomponent.getAbsolutePathString();

OpenSim/Common/Component.h

@@ -55,6 +55,8 @@
 #include <OpenSim/Common/osimCommonDLL.h>
 
 #include <functional>
+#include <memory>
+#include <type_traits>
 #include <unordered_map>
 #include <utility>
 
@@ -670,6 +672,41 @@ OpenSim_DECLARE_ABSTRACT_OBJECT(Component, Object);
     * @param subcomponent is the Component to be added. */
     void addComponent(Component* subcomponent);
 
+#ifndef SWIG
+    /**
+     * Returns `subcomponent` if `subcomponent` was successfully extracted from
+     * this `Component`.
+     *
+     * Specifically, this tries to extract a direct sub-`Component` that was
+     * previously added via `addComponent`, or existed in the `<components>` list
+     * XML of this `Component`.
+     */
+    std::unique_ptr<Component> extractComponent(Component* subcomponent);
+
+    template<
+        typename T,
+        std::enable_if_t<std::is_base_of_v<Component, T>, bool> = true
+    >
+    std::unique_ptr<T> extractComponent(T* subcomponent)
+    {
+        // If the type of `subcomponent` is known at compile-time then it is also
+        // known that the return value will be `subcomponent`, which will have the
+        // same type.
+        std::unique_ptr<Component> typeErased = extractComponent(static_cast<Component*>(subcomponent));
+        return std::unique_ptr<T>{static_cast<T*>(typeErased.release())};
+    }
+#endif
+
+    /**
+     * Returns `true` if `subcomponent`, which should be a direct subcomponent
+     * of this component, was successfully removed from this component.
+     *
+     * Specifically, this tries to remove a direct component that was
+     * previously added via `addComponent`, or existed in the `<components>`
+     * list XML for this component.
+     */
+    bool removeComponent(Component* subcomponent);
+
     /**
      * Get an iterator through the underlying subcomponents that this component is
      * composed of. The hierarchy of Components/subComponents forms a tree.

OpenSim/Common/Property.h

@@ -35,6 +35,7 @@
 #include "SimTKcommon/internal/Array.h"
 #include "SimTKcommon/internal/ClonePtr.h"
 
+#include <memory>
 #include <iomanip>
 #include <set>
 
@@ -519,15 +520,22 @@ class Property : public AbstractProperty {
         return adoptAndAppendValueVirtual(value);
     }
     /** Remove specific entry of the list at index **/
-    void removeValueAtIndex(int index) {
-        if (index > getNumValues() || index < 0)
-            throw OpenSim::Exception("Property<T>::removeValueAtIndex(): Property " + getName()
-                + "invalid index, out of range, ignored.");
-        if (getNumValues() - 1 < this->getMinListSize() || getNumValues() - 1 > this->getMaxListSize())
-            throw OpenSim::Exception("Property<T>::removeValueAtIndex(): Property " + getName()
-                + "resulting list has improper size, ignored.");
-        removeValueAtIndexVirtual(index);
+    void removeValueAtIndex(int index) { extractValueAtIndex(index); }
+
+#ifndef SWIG
+    /** Extract a specific entry of the list at index **/
+    std::unique_ptr<T> extractValueAtIndex(int index)
+    {
+        if (index > getNumValues() || index < 0) {
+            throw OpenSim::Exception("Property<T>::extractValueAtIndex(): Property " + getName() + "invalid index, out of range, ignored.");
+        }
+        if (getNumValues() - 1 < this->getMinListSize() || getNumValues() - 1 > this->getMaxListSize()) {
+            throw OpenSim::Exception("Property<T>::extractValueAtIndex(): Property " + getName() + "resulting list has improper size, ignored.");
+        }
+        return extractValueAtIndexVirtual(index);
     }
+#endif
+
     /** Search the value list for an element that has the given \a value and
     return its index if found, otherwise -1. This requires only that the 
     template type T supports operator==(). This is a linear search so will 
@@ -600,7 +608,7 @@ class Property : public AbstractProperty {
     virtual void setValueVirtual(int index, const T& value) = 0;
     virtual int appendValueVirtual(const T& value) = 0;
     virtual int adoptAndAppendValueVirtual(T* value) = 0;
-    virtual void removeValueAtIndexVirtual(int index) = 0;
+    virtual std::unique_ptr<T> extractValueAtIndexVirtual(int index) = 0;
     /** @endcond **/
 #endif
 };
@@ -1017,8 +1025,17 @@ class SimpleProperty : public Property<T> {
     {   values.push_back(*valuep); // make a copy
         delete valuep; // throw out the old one
         return values.size()-1; }
-    void removeValueAtIndexVirtual(int index) override final
-    {   values.erase(&values[index]);  }
+
+#ifndef SWIG
+    std::unique_ptr<T> extractValueAtIndexVirtual(int index) override final
+    {
+        T* p = &values.at(index);
+        auto extracted = std::make_unique<T>(*p);
+        values.erase(p);
+        return extracted;
+    }
+#endif
+
     // This is the default implementation; specialization is required if
     // the Simbody default behavior is different than OpenSim's; e.g. for
     // Transform serialization.
@@ -1195,10 +1212,17 @@ class ObjectProperty : public Property<T> {
                 return i;
         return -1;
     }
-    // Remove value at specific index
-    void removeValueAtIndexVirtual(int index) override {
-        objects.erase(&objects.at(index));
+
+#ifndef SWIG
+    // Extract (remove and return) a value at a specific index
+    std::unique_ptr<T> extractValueAtIndexVirtual(int index) override
+    {
+        SimTK::ClonePtr<T>* p = &objects.at(index);
+        std::unique_ptr<T> extracted{p->release()};
+        objects.erase(p);
+        return extracted;
     }
+#endif
 private:
     // Base class checks the index.
     const T& getValueVirtual(int index) const override final 

OpenSim/Common/Test/testComponentInterface.cpp

@@ -78,6 +78,18 @@ namespace
             constructProperty_func();
         }
     };
+
+    class BlankComponent final : public OpenSim::Component {
+        OpenSim_DECLARE_CONCRETE_OBJECT(BlankComponent, OpenSim::Component);
+    };
+
+    class ComponentWithObjectProperty final : public OpenSim::Component {
+        OpenSim_DECLARE_CONCRETE_OBJECT(ComponentWithObjectProperty, OpenSim::Component);
+    public:
+        OpenSim_DECLARE_PROPERTY(object_property, BlankComponent, "an object property");
+
+        ComponentWithObjectProperty() { constructProperty_object_property(BlankComponent{}); }
+    };
 }
 
 using namespace OpenSim;
@@ -3413,6 +3425,51 @@ TEST_CASE("Component Interface: Incorrectly Reading an Optional Object Property
     }
 }
 
+TEST_CASE("Component Interface: removeComponent returns `true` if it removes a component added via addComponent")
+{
+    BlankComponent c;
+    BlankComponent* child = new BlankComponent{};
+    c.addComponent(child);
+    CHECK(c.removeComponent(child));
+}
+
+TEST_CASE("Component Interface: removeComponent returns `false` if given a `Component` that wasn't added via addComponent")
+{
+    BlankComponent c;
+    auto unrelated = std::make_unique<BlankComponent>();
+    CHECK(!c.removeComponent(unrelated.get()));
+}
+
+TEST_CASE("Component Interface: removeComponent returns `false` if caller tries to remove a direct object property")
+{
+    ComponentWithObjectProperty c;
+    Component* property = &c.upd_object_property();
+    CHECK(!c.removeComponent(property));
+}
+
+TEST_CASE("Component Interface: extractComponent returns the component added via addComponent")
+{
+    BlankComponent c;
+    BlankComponent* child = new BlankComponent{};
+    c.addComponent(child);
+    auto extracted = c.extractComponent(child);
+    CHECK(extracted.get() == child);
+}
+
+TEST_CASE("Component Interface: extractComponent returns nullptr if given a `Component` that wasn't added via addComponent")
+{
+    BlankComponent c;
+    auto unrelated = std::make_unique<BlankComponent>();
+    CHECK(c.extractComponent(unrelated.get()) == nullptr);
+}
+
+TEST_CASE("Component Interface: extractComponent returns `nullptr` if caller tries to remove a direct object property")
+{
+    ComponentWithObjectProperty c;
+    Component* property = &c.upd_object_property();
+    CHECK(c.extractComponent(property).get() == nullptr);
+}
+
 const bool g_TestFixtureTypesAreRegistered = []()
 {
     // ensure new types are globally registered for testing deserialization