2

我的代码中最常见的错误之一是 STL 容器在循环期间被修改。

在循环执行期间删除或添加元素，因此我通常会遇到超出范围的异常。

我的 for 循环通常如下所示：

for (auto& Item : Items) { // Will not work when Items container is modified
    //... loop logic
}

当可以删除多个项目时，我使用这个怪物：

for (int Index=Items.size()-1;Index<=0;Index--) {
    if (Index<Items.size()) { //Because multiple items can be removed in a single loop
        //... loop logic
    }
}

这看起来很糟糕，使用第二个选项让我感觉很糟糕。可以删除多个项目的原因是由于事件，其中单个事件可以删除任意数量的元素。

以下是一些伪代码来说明何时发生这种情况：

// for each button in vector<button> {
// process button events
// event adds more buttons to vector<button>
// *ERROR* vector<button> is modified during loop.
// }

在另一个示例中，想象一个包含以下项目的向量：

// 0 1 2 3 4 5 6 7 8 9

我们开始循环0并逐个元素进行。在4，我想删除 elements 1，4所以9我们不能在这里使用正常的循环。

4

4 回答 4

6

std::remove_if与决定是否需要删除按钮的谓词一起使用：

bool needsRemoved(const Button& button);

vec.erase(std::remove_if(vec.begin(), vec.end(), &needsRemoved), vec.end());

编辑：对于你的最后一个例子，二次（即对性能不利）算法是：

std::vector<int> vec = {0,1,2,3,4,5,6,7,8,9};
auto end = vec.end();
for (auto it = vec.begin(); it < end; ++it)
{
    std::set<int> bad = {1, 4, 9};
    end = std::remove_if
        (vec.begin(), end,
         [bad](int x) { return (bad.find(x) != bad.end()); });
}
vec.erase(end, vec.end());

不过，使用快速查找的容器（如集合或地图）可能会更好。

于 2013-06-21T08:47:20.880 回答
3

几乎有两种方法可以可靠地做到这一点：

  1. 遍历原始容器的副本并操作原始容器。除非您的容器存储指针，而不是直接存储实际元素，否则这可能不可行。

  2. 不允许直接操作容器，而是以某种方式标记要删除的元素并在迭代后清除它们。您还可以通过将新元素插入单独的临时容器并在循环完成后附加到原始容器来支持添加新元素 - 您也可以对已删除的元素执行此操作，从而无需在元素本身中存储“已删除”标志. 这当然可以用合适add的remove函数抽象出来。

编辑：解决方案＃2的删除部分可以很好地使用rectummelancolique所示的擦除删除成语完成。

于 2013-06-21T08:45:23.310 回答
0

由于您在谈论按钮和按钮事件：最简单的解决方案是在处理事件时将循环重置为开始：

for ( auto current = items.begin(); current != items.end(); ++ current ) {
    if ( current->hasEventWhichNeedsProcessing() ) {
        current->processEvent();    //  possibly invalidates current
        current = items.begin();    //  revalidates current
    }
}

如果我们谈论的是按钮事件（由于人类用户操作而发生），这应该是安全的，因为您通常应该能够在新事件发生之前处理所有事件。（对于非常迅速发生的事件，您可能永远无法到达最终条目。）

但是，我仍然不确定这是最好的解决方案。无论您如何迭代，这意味着您可以按照与事件到达不同的顺序来处理事件。更好的解决方案是将事件本身推送到列表中，然后按顺序处理此列表（作为队列）。

于 2013-06-21T09:02:09.103 回答
0

Since there are buttons (and I hope there are not too many) you might want to add a flag to each button, which tells, if it has been processed completely or something like that. Then you look for the first item in the array which has not been processed and process it. You repeat this until all items have been processed. 

for (;;) // breaks, when all items have been processed.
{
    auto it = std::find( std::begin(Items), std::end(Items), 
        [](const Item & item){ return item.hasBeenProcessed(); }
    if ( it == std::end(Items) )
        break;
    process( *it );
}

This should be safe. Note that this can have quadratic time complexity with respect to the number of items. As I said, there will hopefully not be too many items. If this is an issue, you might want to optimize this loop a little, for example starting the search where you left the last time. But do this only when it becomes an issue.

于 2013-06-21T08:42:38.033 回答