Remove return value from Container::insert

adapted Container::add

Logical consequence of removing the ROS_ERROR output in `setParent`
This commit is contained in:
Robert Haschke 2021-02-22 19:42:28 +01:00 committed by Michael Görner
parent 4386d0cc60
commit a6fa45214f
6 changed files with 17 additions and 24 deletions

View File

@ -64,7 +64,7 @@ public:
void add(Stage::pointer&& stage); void add(Stage::pointer&& stage);
virtual bool insert(Stage::pointer&& stage, int before = -1); virtual void insert(Stage::pointer&& stage, int before = -1);
virtual Stage::pointer remove(int pos); virtual Stage::pointer remove(int pos);
virtual Stage::pointer remove(Stage* child); virtual Stage::pointer remove(Stage* child);
virtual void clear(); virtual void clear();
@ -205,7 +205,7 @@ public:
WrapperBase(const std::string& name = "wrapper", Stage::pointer&& child = Stage::pointer()); WrapperBase(const std::string& name = "wrapper", Stage::pointer&& child = Stage::pointer());
/// insertion is only allowed if children() is empty /// insertion is only allowed if children() is empty
bool insert(Stage::pointer&& stage, int before = -1) override; void insert(Stage::pointer&& stage, int before = -1) override;
/// access the single wrapped child, NULL if still empty /// access the single wrapped child, NULL if still empty
Stage* wrapped(); Stage* wrapped();

View File

@ -112,13 +112,12 @@ public:
/// set parent of stage /// set parent of stage
/// enforce only one parent exists /// enforce only one parent exists
inline bool setParent(ContainerBase* parent) { inline void setParent(ContainerBase* parent) {
if (parent_) { if (parent_) {
// it's not allowed to add a stage to a parent if it already has one // it's not allowed to add a stage to a parent if it already has one
throw std::runtime_error("Tried to add stage '" + name() + "' to two parents"); throw std::runtime_error("Tried to add stage '" + name() + "' to two parents");
} }
parent_ = parent; parent_ = parent;
return true;
} }
/// explicitly orphan stage /// explicitly orphan stage

View File

@ -94,7 +94,7 @@ public:
void loadRobotModel(const std::string& robot_description = "robot_description"); void loadRobotModel(const std::string& robot_description = "robot_description");
void add(Stage::pointer&& stage); void add(Stage::pointer&& stage);
bool insert(Stage::pointer&& stage, int before = -1) override; void insert(Stage::pointer&& stage, int before = -1) override;
void clear() final; void clear() final;
/// enable introspection publishing for use with rviz /// enable introspection publishing for use with rviz

View File

@ -182,24 +182,18 @@ bool ContainerBase::traverseRecursively(const ContainerBase::StageCallback& proc
} }
void ContainerBase::add(Stage::pointer&& stage) { void ContainerBase::add(Stage::pointer&& stage) {
if (!insert(std::move(stage))) { insert(std::move(stage), -1);
throw std::runtime_error(name() + ": Could not insert stage");
}
} }
bool ContainerBase::insert(Stage::pointer&& stage, int before) { void ContainerBase::insert(Stage::pointer&& stage, int before) {
if (!stage) { if (!stage)
ROS_ERROR_STREAM(name() << ": received invalid stage pointer"); throw std::runtime_error(name() + ": received invalid stage pointer");
return false;
}
StagePrivate* impl = stage->pimpl(); StagePrivate* impl = stage->pimpl();
if (!impl->setParent(this)) impl->setParent(this);
return false;
ContainerBasePrivate::const_iterator where = pimpl()->childByIndex(before, true); ContainerBasePrivate::const_iterator where = pimpl()->childByIndex(before, true);
ContainerBasePrivate::iterator it = pimpl()->children_.insert(where, std::move(stage)); ContainerBasePrivate::iterator it = pimpl()->children_.insert(where, std::move(stage));
impl->setParentPosition(it); impl->setParentPosition(it);
return true;
} }
Stage::pointer ContainerBasePrivate::remove(ContainerBasePrivate::const_iterator pos) { Stage::pointer ContainerBasePrivate::remove(ContainerBasePrivate::const_iterator pos) {
@ -636,10 +630,10 @@ WrapperBase::WrapperBase(WrapperBasePrivate* impl, Stage::pointer&& child) : Par
WrapperBase::insert(std::move(child)); WrapperBase::insert(std::move(child));
} }
bool WrapperBase::insert(Stage::pointer&& stage, int before) { void WrapperBase::insert(Stage::pointer&& stage, int before) {
// restrict num of children to one // restrict num of children to one
if (numChildren() > 0) if (numChildren() > 0)
return false; throw std::runtime_error(name() + ": Wrapper only allows a single child");
return ParallelContainerBase::insert(std::move(stage), before); return ParallelContainerBase::insert(std::move(stage), before);
} }

View File

@ -203,8 +203,8 @@ void Task::add(Stage::pointer&& stage) {
stages()->add(std::move(stage)); stages()->add(std::move(stage));
} }
bool Task::insert(Stage::pointer&& stage, int before) { void Task::insert(Stage::pointer&& stage, int before) {
return stages()->insert(std::move(stage), before); stages()->insert(std::move(stage), before);
} }
void Task::clear() { void Task::clear() {

View File

@ -298,28 +298,28 @@ TEST_F(SerialTest, insertion_order) {
/***** inserting first stage *****/ /***** inserting first stage *****/
auto g = std::make_unique<GeneratorMockup>(); auto g = std::make_unique<GeneratorMockup>();
StagePrivate* gp = g->pimpl(); StagePrivate* gp = g->pimpl();
ASSERT_TRUE(container.insert(std::move(g))); container.insert(std::move(g));
EXPECT_FALSE(g); // ownership transferred to container EXPECT_FALSE(g); // ownership transferred to container
VALIDATE(gp); VALIDATE(gp);
/***** inserting second stage *****/ /***** inserting second stage *****/
auto f = std::make_unique<ForwardMockup>(); auto f = std::make_unique<ForwardMockup>();
StagePrivate* fp = f->pimpl(); StagePrivate* fp = f->pimpl();
ASSERT_TRUE(container.insert(std::move(f))); container.insert(std::move(f));
EXPECT_FALSE(f); // ownership transferred to container EXPECT_FALSE(f); // ownership transferred to container
VALIDATE(gp, fp); VALIDATE(gp, fp);
/***** inserting third stage *****/ /***** inserting third stage *****/
auto f2 = std::make_unique<ForwardMockup>(); auto f2 = std::make_unique<ForwardMockup>();
StagePrivate* fp2 = f2->pimpl(); StagePrivate* fp2 = f2->pimpl();
ASSERT_TRUE(container.insert(std::move(f2), 1)); container.insert(std::move(f2), 1);
EXPECT_FALSE(f2); // ownership transferred to container EXPECT_FALSE(f2); // ownership transferred to container
VALIDATE(gp, fp2, fp); VALIDATE(gp, fp2, fp);
/***** inserting another generator stage *****/ /***** inserting another generator stage *****/
auto g2 = std::make_unique<GeneratorMockup>(); auto g2 = std::make_unique<GeneratorMockup>();
StagePrivate* gp2 = g2->pimpl(); StagePrivate* gp2 = g2->pimpl();
ASSERT_TRUE(container.insert(std::move(g2))); container.insert(std::move(g2));
VALIDATE(gp, fp2, fp, gp2); VALIDATE(gp, fp2, fp, gp2);
} }