如何通过 return 语句更改指针指向的位置？

Question

我有一个包含 ID 字符串的 Player 对象。我试图通过遍历玩家列表来检查玩家是否已经存在。如果播放器已经存在，我想返回现有播放器，否则我将创建一个 Player 对象的新实例。

Player *playerExists(const std::string &playerId, const std::vector<Player>* players)
{
    for (Player existingPlayer : *players)
    {
        if (existingPlayer.playerId == playerId)
        {
            std::cout << "Player " << existingPlayer.name << " exists" << std::endl;
            return &existingPlayer;
        }
    }
    return new Player();
}

问题似乎出在 return 语句中。我不知道如何将特定对象返回给指针。这似乎是我发现不会出错的唯一方法（谈论 return 语句中的 & sing）。

Player* player = playerExists("SomeID", listOfPlayers);
listOfPlayers->push_back(*player);
delete player;

我对使用原始指针很陌生，所以我可能只是不明白这里的问题是什么。如果有人能解释我做错了什么，我将不胜感激。

Answer 1

这段代码有几个问题。

循环的每次迭代都在向量中制作Player 的本地副本，因此如果找到匹配项，将返回指向该副本的指针.但是当当前迭代完成时，那个 copy 被销毁了，所以你最终会返回一个 悬空指针。要解决这个问题，您的循环需要对向量中的每个 Player 进行 reference：

for (Player &existingPlayer : *players)

下一个问题是，如果找不到匹配项，则返回 newed Player。问题在于调用者无条件地deleteing 返回的Player*，无论它是否是newed。向量拥有它所持有的Player 对象，因此delete 使用其中一个对象是未定义的行为。

如果没有找到匹配项，更好的设计选择是让playerExists() 返回nullptr。然后，调用者可以根据返回的是 Player 还是 nullptr 执行不同的操作，例如：

Player* playerExists(const std::string &playerId, const std::vector<Player>* players)
{
    for (Player &existingPlayer : *players)
    {
        if (existingPlayer.playerId == playerId)
        {
            std::cout << "Player " << existingPlayer.name << " exists" << std::endl;
            return &existingPlayer;
        }
    }
    return nullptr;
}

Player* player = playerExists("SomeID", listOfPlayers);
if (player) {
    // ID found, use player as needed...
    listOfPlayers->push_back(*player); // why re-add a player that is already in the list???
} else {
    // ID not found, do something else...
    listOfPlayers->push_back(Player{}); // why add a blank player???
}

Answer 2

解决此问题的不同方法。最大的问题可能在这一行：

for (Player existingPlayer : *players)

它正在复制（我认为）收藏中已有的玩家。

当您执行return &existingPlayer; ... 时，它会返回堆栈上的一个对象。坏事会发生。

你最好迭代集合并返回一个迭代器，恕我直言......也许......

std::vector<Player>::const_iterator playerExists(const std::string &playerId, const std::vector<Player>& players)
{
    std::vector<Player>::const_iterator it = players.begin();
    for (; it != players.end(); ++it)
    {
        if (it->playerId == playerId)
        {
            std::cout << "Player " << it->name << " exists" << std::endl;
            break;
        }
    }
    return it;
}

你可以在返回后随时测试

使用返回的迭代器，你必须对它做一些事情......

auto it = playerExists(playerId, players);
if (it == players.end())
{
   // not found
} else {
   // found
}

Answer 3

你的问题在那里：

for (Player existingPlayer : *players)

在 for 循环的每次迭代中，existingPlayer 是播放器的 副本，它存储在 for 范围内的局部变量中 循环。您正在返回一个指向局部变量的指针，该变量在每次循环迭代后被销毁。

要避免复制，请使用 reference ：

for (Player &existingPlayer : *players)

这里，existingPlayer 是对真实玩家的参考。这就是你对函数原型中的const std::string 所做的。

我还有一些建议可以改进您的代码：

• 避免使用原始 C 指针，C++ 为您提供了工具，以一种（更）不易出错的方式获得相同的功能，因此更安全。如果您不需要 NULL 正如我在您的代码中看到的那样，那么参考可能会更好。否则，您应该考虑使用智能指针（std::unique_ptr、std::shared_ptr。在您的情况下，我会通过引用而不是指针来传递 players。

• std::vector 可能不是最好的容器。 选择正确的容器可能很困难。 目前您正在遍历整个数组以通过其 id 找到玩家。您应该使用使用哈希表的std::unordered_map<std::string,Player>，并且可能会加快查找时间。 我也会使用std::a_container_class<Player> 的 typedef 来避免在您更改容器类型或稍后从头开始编写容器时重写大量代码。

• 复制Player 有意义吗？ 如果没有，请显式删除copy constructor 和copy assignment operator，如果您要复制Player，编译器会抛出错误。 请注意，这是一种设计选择，我经常这样做，同一播放器的多个实例是正常的，具体取决于您的对象设计。

• 我对@9​​87654338@ 持怀疑态度。如果没有玩家存在，为什么要返回新玩家？根据我个人的解释（可能完全错误），我会返回一个迭代器。不涉及指针，您可以“取消引用”迭代器以获取对找到的玩家的引用，并且通过返回等于players.end() 的迭代器来表示玩家不存在。但要注意一个很大的缺点，一定要在修改容器和使迭代器失效之前进行测试。

根据我自己的设计选择，我为您的函数制作了一个未经测试且较短的版本：

#include <unordered_map>
typedef std::unordered_map<std::string,Player> PlayerCollection;
/* ... */
PlayerCollection::const_iterator playerExists(const std::string &playerId, const PlayerCollection &players)
{
    return players.find(playerId);
}

请记住，我的建议是固执己见，并非绝对真理。 阅读其他有不同想法的答案。