0

我想知道这是否是一种可接受的做法：

struct Item { };
std::list<std::shared_ptr<Item>> Items;
std::list<std::shared_ptr<Item>> RemovedItems;

void Update()
{
    Items.push_back(std::make_shared<Item>()); // sample item

    for (auto ItemIterator=Items.begin();ItemIterator!=Items.end();ItemIterator++)
    {
        if (true) { // a complex condition, (true) is for demo purposes
            RemovedItems.push_back(std::move(*ItemIterator)); // move ownership
            *ItemIterator=nullptr; // set current item to nullptr
        }

        // One of the downsides, is that we have to always check if
        // the current iterator value is not a nullptr
        if (*ItemIterator!=nullptr) {
            // A complex loop where Items collection could be modified
        }
    }

    // After the loop is done, we can now safely remove our objects

    RemovedItems.clear(); // calls destructors on objects

    //finally clear the items that are nullptr
    Items.erase( std::remove_if( Items.begin(), Items.end(),
        [](const std::shared_ptr<Item>& ItemToCheck){
            return ItemToCheck==nullptr;
    }), Items.end() );
}

这里的想法是我们正在标记 Items 容器可能会受到外部来源的影响。当从容器中删除一个项目时，它只是设置为 nullptr 但在此之前移动到 RemovedItems。

像事件这样的东西可能会影响Items和添加/删除项目，所以我不得不想出这个解决方案。

这看起来是个好主意吗？

4

3 回答 3

2

我认为你把事情复杂化了。如果您处于多线程情况（您在问题中没有提到它），您肯定需要一些锁来保护来自访问您修改列表的其他线程的读取。由于标准库中没有并发数据结构，因此您需要自己添加这些内容。

对于单线程代码，您可以简单地使用谓词调用std:list成员。remove_if无需将指针设置为 null、存储它们并对数据进行多次传递。

#include <algorithm>
#include <list>
#include <memory>
#include <iostream>

using Item = int;

int main()
{
    auto lst = std::list< std::shared_ptr<Item> > 
    { 
        std::make_shared<int>(0), 
        std::make_shared<int>(1), 
        std::make_shared<int>(2), 
        std::make_shared<int>(3),         
    };    

    // shared_ptrs to even elements
    auto x0 = *std::next(begin(lst), 0);
    auto x2 = *std::next(begin(lst), 2);

    // erase even numbers
    lst.remove_if([](std::shared_ptr<int> p){
        return *p % 2 == 0;    
    });

    // even numbers have been erased
    for (auto it = begin(lst); it != end(lst); ++it)
        std::cout << **it << ",";    
    std::cout << "\n";

    // shared pointers to even members are still valid
    std::cout << *x0 << "," << *x2;
}

活生生的例子。

请注意，这些元素实际上已从列表中删除，而不仅仅是放在列表的末尾。后一种效果是标准算法std::remove_if会做的事情，之后你必须调用std::list成员函数erase。这个两步擦除删除成语看起来像这样

// move even numbers to the end of the list in an unspecified state
auto res = std::remove_if(begin(lst), end(lst), [](std::shared_ptr<int> p){
    return *p % 2 == 0;    
});

// erase even numbers
lst.erase(res, end(lst));

活生生的例子。

然而，在这两种情况下，底层Item元素都没有被删除，因为它们每个都有一个与之关联的共享指针。只有当引用计数下降到零时，才会真正删除那些以前的列表元素。

于 2013-10-07T12:30:07.017 回答
1

如果我正在查看此代码，我会说这是不可接受的。

两阶段去除的目的是什么？像这样一个不寻常的决定需要评论来解释其目的。尽管一再要求你未能解释它的重点。

这里的想法是我们标记Items容器可能会受到外部来源的影响。

你的意思是“这里的想法是，当我们标记物品时，容器可能会受到外部来源的影响。” ? 否则这句话没有意义。

怎么可能受到影响？你的解释不清楚：

想想Root -> Parent -> Child关系。一个事件可能会在一个Child可以Parent从中删除的事件中触发Root。所以循环可能会在中间中断，迭代器将无效。

这并不能解释任何事情，它太模糊了，使用了非常广泛的术语。解释你的意思。

“亲子关系”可能意味着很多不同的事情。你的意思是类型是相关的，通过继承？对象是相关的，按所有权？什么？

什么样的“事件”？事件可能意味着很多事情，我希望 StackOverflow 上的人们停止使用“事件”这个词来表示特定的事情，并假设其他人都知道他们想要什么意思。您的意思是异步事件，例如在另一个线程中？或者你的意思是破坏一个Item可能会导致从Items列表中删除其他元素？

如果您指的是异步事件，那么您的解决方案完全无法解决问题。如果可以同时修改该容器，则您无法安全地迭代任何标准容器。为了确保安全，您必须做一些事情（例如锁定互斥锁）以确保在修改容器时独占访问容器。

基于此评论：

// A complex loop where Items collection could be modified

我假设您不是指异步事件（但是为什么说“外部源”可以改变容器）在这种情况下，您的解决方案确实确保迭代器在“复杂循环”迭代列表时保持有效，但是为什么是否需要实际Item对象保持有效，而不仅仅是保持迭代器有效？难道你不能把元素设置为nullptr 而不把它放进去RemovedItems，然后Items.remove_if([](shared_ptr<Item> const& p) { return !p; }在最后做吗？您需要更多地解释一下您的“复杂循环”可以对容器或项目做什么。

为什么函数中RemovedItems没有局部变量Update()？在该功能之外似乎不需要它。为什么不使用新的 C++11 基于范围的for循环来遍历列表？

最后，为什么所有东西都用大写字母命名？！用大写字母命名局部变量和函数很奇怪，如果所有内容都这样命名，那么它就毫无意义，因为大写字母无助于区分不同类型的名称（例如，仅对类型使用大写字母可以明确哪些名称是类型和哪些不是......将它用于一切都是无用的。）

于 2013-10-07T14:16:36.160 回答
-2

我觉得这只会使事情变得复杂，因为必须到处检查 nullptr。此外，移动 shared_ptr 有点傻。

编辑：

我想我现在明白了这个问题，这就是我将如何解决它：

struct Item {
    std::list<std::shared_ptr<Item>> Children;
    std::set < std::shared_ptr<Item>, std::owner_less < std::shared_ptr<Item >> > RemovedItems;
    void Update();
    void Remove(std::shared_ptr<Item>);
};

void Item::Update()
{
    for (auto child : Children){
        if (true) { // a complex condition, (true) is for demo purposes
            RemovedItems.insert(child);
        }
        // A complex loop where children collection could be modified but
        // only by calling Item::remove, Item::add or similar
    }
    auto oless = std::owner_less < std::shared_ptr < Item >>();
    std::sort(Children.begin(), Children.end(), oless ); //to avoid use a set

    auto newEnd = std::set_difference(Children.begin(),
        Children.end(),
        RemovedItems.begin(),
        RemovedItems.end(),
        Children.begin(),
        oless);
    Children.erase(newEnd, Children.end());

    RemovedItems.clear(); // may call destructors on objects

}

void Item::Remove(std::shared_ptr<Item> element){
    RemovedItems.insert(element);
}
于 2013-10-07T09:42:24.067 回答