英文:
C++ - Error while iterating std::list and deleting element
问题
以下是您要翻译的代码部分:
This is my code:
std::list<GameMatch*> games_;
void GameMatchManager::ClearEndedGames()
{
if(games_.empty())
return;
auto it = games_.begin();
while (it != games_.end())
{
auto game = *it;
if(game->MatchEnded())
{
games_.erase(it);
game->Destroy();
}
++it;
}
}
void GameMatch::Destroy()
{
std::cout << "Destoying " << GetMatchId() << std::endl;
delete this;
}
This is the error I am getting:
[![enter image description here][1]][1]
Why am I getting this error in first place when at the top i do `if(games.empty()) return;` ?
Shouldn't that stop the iteration of `games_` list ?
Why am I getting this error and how can I fix it?
希望这有助于您解决问题。
英文:
This is my code:
std::list<GameMatch*> games_;
void GameMatchManager::ClearEndedGames()
{
if(games_.empty())
return;
auto it = games_.begin();
while (it != games_.end())
{
auto game = *it;
if(game->MatchEnded())
{
games_.erase(it);
game->Destroy();
}
++it;
}
}
void GameMatch::Destroy()
{
std::cout << "Destoying " << GetMatchId() << std::endl;
delete this;
}
This is the error I am getting:
Why am I getting this error in first place when at the top i do if(games.empty()) return;
?
Shouldn't that stop the iteration of games_
list ?
Why am I getting this error and how can I fix it?
答案1
得分: 1
明显的问题:您正在使用无效的迭代器。当您从std::list
中删除一个元素时,指向其他元素的迭代器仍然有效,但被删除的元素的迭代器处于未定义状态。一个可能的解决方案是在删除之前先迭代一步:
template <typename List>
void ConsumeList(List&& list) {
auto last_it = list.begin();
if (last_it != list.end()) {
for (auto it = std::next(last_it); it != list.end(); ++it) {
list.erase(last_it); // 此处 last_it 变为无效状态。
last_it = it; // 没关系,我们重新使其有效。
}
list.erase(last_it); // 不要忘记这个!
}
}
其他注意事项,大部分与问题无关:
- 避免使用
Destroy()
等方法;它可能执行的是应该由~GameMatch()
执行的操作,但没有来自编译器的强有关的RAII保证。优先选择RAII而不是“自制”资源管理。 - 不要使用裸指针。您几乎永远不需要它们。
- 最好根本不直接分配对象(使用智能指针或其他方式)。让STL容器处理分配。直接嵌入对象(
std::list<GameMatch>
而不是std::list<GameMatch*>
)。指针跳跃越少,效率越高。
这是ConsumeList()
的示例,这次在一个可构建和可运行上下文中的代码:
#include <iostream>
#include <iterator>
#include <list>
#include <memory>
#include <string>
#include <string_view>
#include <utility>
namespace {
struct GameMatch {
GameMatch(std::string_view match_id) : match_id_{match_id} {}
std::string_view GetMatchId() const { return match_id_; }
~GameMatch() { std::cout << "Destoying " << GetMatchId() << std::endl; }
private:
const std::string match_id_;
};
template <typename List>
void ConsumeList(List&& list) {
auto last_it = list.begin();
if (last_it != list.end()) {
for (auto it = std::next(last_it); it != list
<details>
<summary>英文:</summary>
The obvious problem: You are using an invalid iterator. When you delete an element from a `std::list`, iterators pointing at **other** elements remain valid, but the deleted element’s iterator is in an undefined state. A possible solution is to iterate one step ahead of deletions:
```c++
template <typename List>
void ConsumeList(List&& list) {
auto last_it = list.begin();
if (last_it != list.end()) {
for (auto it = std::next(last_it); it != list.end(); ++it) {
list.erase(last_it); // last_it becomes invalid here.
last_it = it; // Never mind, we make it valid again.
}
list.erase(last_it); // Don't forget this!
}
}
Other notes, mostly unrelated to the problem:
- Avoid methods such as
Destroy()
; it is likely doing something that~GameMatch()
should be doing instead, but without strong RAII-related guarantees from the compiler. Prefer RAII over “homebrew” resource management. - Do not use raw pointers. You almost never(*) need them.
(*) The only exception is when you implement low-level linked data structures. Then (and only then) it makes sense to use raw pointers. - Preferably, do not allocate objects directly (using smart pointers or otherwise) at all. Instead, let STL containers handle allocation. Embed your objects directly (
std::list<GameMatch>
instead ofstd::list<GameMatch*>
). The fewer pointer jumps, the better efficiency you get.
Here’s ConsumeList()
once again, this time in a buildable and runnable context:
#include <iostream>
#include <iterator>
#include <list>
#include <memory>
#include <string>
#include <string_view>
#include <utility>
namespace {
struct GameMatch {
GameMatch(std::string_view match_id) : match_id_{match_id} {}
std::string_view GetMatchId() const { return match_id_; }
~GameMatch() { std::cout << "Destoying " << GetMatchId() << std::endl; }
private:
const std::string match_id_;
};
template <typename List>
void ConsumeList(List&& list) {
auto last_it = list.begin();
if (last_it != list.end()) {
for (auto it = std::next(last_it); it != list.end(); ++it) {
list.erase(last_it);
last_it = it;
}
list.erase(last_it);
}
}
} // namespace
int main() {
std::list<GameMatch> embedded_list;
embedded_list.emplace_back("Embedded Match A");
embedded_list.emplace_back("Embedded Match B");
embedded_list.emplace_back("Embedded Match C");
std::list<std::unique_ptr<GameMatch>> pointer_list;
pointer_list.emplace_back(std::make_unique<GameMatch>("Allocated Match A"));
pointer_list.emplace_back(std::make_unique<GameMatch>("Allocated Match B"));
pointer_list.emplace_back(std::make_unique<GameMatch>("Allocated Match C"));
ConsumeList(std::move(embedded_list));
ConsumeList(std::move(pointer_list));
}
Notice that forgetting to run ConsumeList()
will not cause any memory leaks at the end and all list elements will be properly deallocated. That is the key advantage of using RAII and avoiding raw pointers.
通过集体智慧和协作来改善编程学习和解决问题的方式。致力于成为全球开发者共同参与的知识库,让每个人都能够通过互相帮助和分享经验来进步。
评论