From 84f5c481be4ca9c939560a7220f6ac6a1449ad12 Mon Sep 17 00:00:00 2001 From: Robert Haschke Date: Tue, 13 Mar 2018 12:24:57 +0100 Subject: [PATCH] ContainerBasePrivate::position(): generically handle shifting --- .../moveit/task_constructor/container_p.h | 5 +-- core/src/container.cpp | 14 ++++---- core/test/test_container.cpp | 36 +++++++++---------- 3 files changed, 28 insertions(+), 27 deletions(-) diff --git a/core/include/moveit/task_constructor/container_p.h b/core/include/moveit/task_constructor/container_p.h index 48af8526..3b3e61b6 100644 --- a/core/include/moveit/task_constructor/container_p.h +++ b/core/include/moveit/task_constructor/container_p.h @@ -73,9 +73,10 @@ public: inline const container_type& children() const { return children_; } /** Retrieve iterator into children_ pointing to indexed element. - * Negative index counts from end(), i.e. -1 is end(), -2 is --end(), etc. + * Negative index counts from end, i.e. -1 is last, -2 is second last, etc. (if !for_insert) + * If for_insert == true, negative indexes are shifted by one: -1 is end(), -2 is --end(), etc. * Contrary to std::advance(), iterator limits are considered. */ - const_iterator position(int index) const; + const_iterator position(int index, bool for_insert = false) const; /// traversing all stages up to max_depth bool traverseStages(const ContainerBase::StageCallback &processor, diff --git a/core/src/container.cpp b/core/src/container.cpp index 5561c18c..ca00294c 100644 --- a/core/src/container.cpp +++ b/core/src/container.cpp @@ -56,7 +56,9 @@ ContainerBasePrivate::ContainerBasePrivate(ContainerBase *me, const std::string pending_forward_.reset(new Interface); } -ContainerBasePrivate::const_iterator ContainerBasePrivate::position(int index) const { +ContainerBasePrivate::const_iterator ContainerBasePrivate::position(int index, bool for_insert) const { + if (!for_insert && index < 0) + --index; const_iterator position = children_.begin(); if (index > 0) { for (auto end = children_.end(); index > 0 && position != end; --index) @@ -164,7 +166,7 @@ bool ContainerBase::insert(Stage::pointer &&stage, int before) return false; } - ContainerBasePrivate::const_iterator where = pimpl()->position(before); + ContainerBasePrivate::const_iterator where = pimpl()->position(before, true); ContainerBasePrivate::iterator it = pimpl()->children_.insert(where, std::move(stage)); impl->setHierarchy(this, it); return true; @@ -172,7 +174,7 @@ bool ContainerBase::insert(Stage::pointer &&stage, int before) bool ContainerBase::remove(int pos) { - ContainerBasePrivate::const_iterator it = pimpl()->position(pos); + ContainerBasePrivate::const_iterator it = pimpl()->position(pos, false); (*it)->pimpl()->setHierarchy(nullptr, ContainerBasePrivate::iterator()); pimpl()->children_.erase(it); return true; @@ -186,8 +188,7 @@ void ContainerBase::clear() void ContainerBase::exposePropertiesOfChild(int child, const std::initializer_list& names) { auto impl = pimpl(); - // for negative child index, return last child for -1, next to last for -2, etc - ContainerBasePrivate::const_iterator child_it = impl->position(child < 0 ? child-1 : child); + ContainerBasePrivate::const_iterator child_it = impl->position(child, false); if (child_it == impl->children().end()) throw std::runtime_error("invalid child index"); @@ -202,8 +203,7 @@ void ContainerBase::exposePropertyOfChildAs(int child, const std::string& child_ const std::string& parent_property_name) { auto impl = pimpl(); - // for negative child index, return last child for -1, next to last for -2, etc - ContainerBasePrivate::const_iterator child_it = impl->position(child < 0 ? child-1 : child); + ContainerBasePrivate::const_iterator child_it = impl->position(child, false); if (child_it == impl->children().end()) throw std::runtime_error("invalid child index"); diff --git a/core/test/test_container.cpp b/core/test/test_container.cpp index 71ced529..a6457ed0 100644 --- a/core/test/test_container.cpp +++ b/core/test/test_container.cpp @@ -73,30 +73,30 @@ public: } }; -TEST(ContainerBase, position) { +TEST(ContainerBase, positionForInsert) { SerialContainer s; SerialContainerPrivate *impl = s.pimpl(); - EXPECT_EQ(impl->position(0), impl->children().end()); - EXPECT_EQ(impl->position(1), impl->children().end()); - EXPECT_EQ(impl->position(-1), impl->children().end()); - EXPECT_EQ(impl->position(-2), impl->children().end()); + EXPECT_EQ(impl->position(0, true), impl->children().end()); + EXPECT_EQ(impl->position(1, true), impl->children().end()); + EXPECT_EQ(impl->position(-1, true), impl->children().end()); + EXPECT_EQ(impl->position(-2, true), impl->children().end()); s.insert(std::make_unique("0")); - EXPECT_STREQ((*impl->position(0))->name().c_str(), "0"); - EXPECT_EQ(impl->position(-1), impl->children().end()); - EXPECT_STREQ((*impl->position(-2))->name().c_str(), "0"); - EXPECT_EQ(impl->position(-3), impl->children().end()); + EXPECT_STREQ((*impl->position(0, true))->name().c_str(), "0"); + EXPECT_EQ(impl->position(-1, true), impl->children().end()); + EXPECT_STREQ((*impl->position(-2, true))->name().c_str(), "0"); + EXPECT_EQ(impl->position(-3, true), impl->children().end()); s.insert(std::make_unique("1")); - EXPECT_STREQ((*impl->position(0))->name().c_str(), "0"); - EXPECT_STREQ((*impl->position(1))->name().c_str(), "1"); - EXPECT_EQ(impl->position(2), impl->children().end()); + EXPECT_STREQ((*impl->position(0, true))->name().c_str(), "0"); + EXPECT_STREQ((*impl->position(1, true))->name().c_str(), "1"); + EXPECT_EQ(impl->position(2, true), impl->children().end()); - EXPECT_EQ(impl->position(-1), impl->children().end()); - EXPECT_STREQ((*impl->position(-2))->name().c_str(), "1"); - EXPECT_STREQ((*impl->position(-3))->name().c_str(), "0"); - EXPECT_EQ(impl->position(-4), impl->children().end()); + EXPECT_EQ(impl->position(-1, true), impl->children().end()); + EXPECT_STREQ((*impl->position(-2, true))->name().c_str(), "1"); + EXPECT_STREQ((*impl->position(-3, true))->name().c_str(), "0"); + EXPECT_EQ(impl->position(-4, true), impl->children().end()); } @@ -154,7 +154,7 @@ protected: ASSERT_TRUE(num == expected.size()) << "different list lengths"; // validate position() - EXPECT_EQ(container->children().begin(), container->position(-(num+1))); + EXPECT_EQ(container->children().begin(), container->position(-num)); EXPECT_EQ(container->children().end(), container->position(num)); // validate order @@ -165,7 +165,7 @@ protected: EXPECT_EQ(child, *exp_it) << "wrong order"; EXPECT_EQ(child->parent()->pimpl(), container) << "wrong parent"; EXPECT_EQ(it, container->position(pos)) << "bad forward position resolution"; - EXPECT_EQ(it, container->position(pos-num-1)) << "bad backward position resolution"; + EXPECT_EQ(it, container->position(pos-num)) << "bad backward position resolution"; } } };