通过 nullptr 在 STD 列表中删除 C++ 标记对象

Question

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

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() );
}

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

事件之类的东西可能会影响Items 并添加/删除项目，所以我必须想出这个解决方案。

这看起来是个好主意吗？

Answer 1

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

对于单线程代码，您可以简单地使用谓词调用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;
}

Live Example.

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

// 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));

Live Example.

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

Answer 2

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

两阶段移除的目的是什么？像这样一个不寻常的决定需要 cmets 解释其目的。尽管一再提出要求，但你未能解释它的意义。

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

你的意思是“这里的想法是虽然我们正在标记项目容器可能会受到外部来源的影响。” ?否则那句话就没有意义了。

它怎么会受到影响？你的解释不清楚：

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

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

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

什么样的“事件”？事件可能意味着很多事情，我希望 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 循环来遍历列表？

最后，为什么everything都用大写字母命名？！用大写字母命名局部变量和函数很奇怪，如果 everything 都是这样命名的，那么它就毫无意义，因为大写字母无助于区分不同类型的名称（例如，仅使用大写字母表示types 明确了哪些名称是类型，哪些不是类型……用它来做所有事情是没有用的。）

Answer 3

我觉得这只会使事情变得复杂，因为必须到处检查 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);
}