需要有关 C++ 中 Table 类的两个成员函数的反馈；

Question

int Table::addPlayer(Player const& player, int position)
{
    if (position > 0 || position < 11) {
        deque<Player>::iterator it = playerList.begin()+position;
        deque<Player>::iterator itStart = playerList.begin()+postion;

        while(*it != "(empty seat)") {
            it++;
            if (it == playerList.end()) {
                it = playerList.begin();
            }
            if (it == itStart) {
cout << "Table full" << endl;
                return -1;
            }
        }
        //TODO overload Player assignment, << operator
        *it = player;
cout << "Player " << player << " sits at position " << it - playerList.begin() << endl;
            return it - playerList.begin();
    } else {
cout << "Position not a valid position, must be 1-10" << endl;
    return -1;
    }
}

int Table::removePlayer(Player const& player)
{
    for (deque<Player>::iterator it = playerList.begin();it != playerList.end(); it++) {
        //TODO Do I need to overload == in Player?
        if(*it == player) {
            *it = "(empty seat)";
            int pos = it - playerList.begin();
cout << "Player " << player << " stands up from position " << pos << endl;
            return pos;
        }
    }
cout << "Player " << player << " not found" << endl;
    return -1;
}

希望获得有关德州扑克模拟的 Table 类的这两个成员函数的一些反馈。任何信息语法、效率甚至常见做法都将不胜感激。

Answer 1

addPlayer() 中的第一个 while 循环取消引用尚未检查有效性的迭代器。如果传入的位置值大于容器中的元素数量，则可能会发生崩溃。这可能由调用者控制，但更好的做法是在参考点控制它。

Answer 2

• 正确缩进代码，可以使以后更容易阅读和理解。如果您缺乏经验，请考虑采用样式指南（例如 Google 的 C++ 样式指南< /a>）。
• 检查玩家迭代器是否取消引用“（空座位）”是有问题的，您可能需要考虑一些替代方案：
  • 保留一个单独的空椅子列表，并在 AddPlayer() 上分配它们。
  • 让多态性发挥作用并使用 EmptySeatPlayer 类。
  • 允许 null 播放器直接存储在 tableList 中。
• 我不清楚为什么 AddPlayer 需要一个 position 参数，如果它只是分配下一个可用座位（直到找到一个）。也许完全删除该参数并让 Table 来计算它。
• 您最终可能希望将游戏文本与业务逻辑分离。
• 您可能不应该直接使用位置，您应该对玩家和桌子进行操作。人们可能会认为它是类的一个细节，不应该由函数公开。
• 使用 std::find，而不是使用 while (*it != player) 并在每次迭代中检查结束。
• 此外，您还进行了大量的“按值传递”，最好传递 const Player& 以避免不必要的复制。

Answer 3

删除可以在 for 循环中完成。

for(deque<Player>::iterator it = playerList.begin(); it!= playerList.end(); it++){
    //if we've found what we're looking for
    if(*it == player){
     //then remove the player and return his/her position.
     *it = "(empty seat)";
     int pos = it - playerList.begin();
     cout << "Player " << player << " stands up from position " << pos << endl;
     return pos;
    }
}
cout << "Player " << player << " not found" << endl;
return -1;

我发现这更干净一些，而且我个人非常喜欢评论。

Answer 4

1) 如果您不打算修改方法中的参数，则通过 const 引用传递：

int Table::addPlayer(Player const& player, int position)

这提供了后者隐藏的好处，但也引入了 const 正确性的概念。

2）尝试并学习如何使用标准算法：

在这种情况下，您的循环可以替换为使用 std::find()

int Table::addPlayer(Player const& player, int position)
{       
    deque<Player>::iterator itStart = playerList.begin()+position;

    deque<Player>::iterator it      = std::find(itStart, playerList.end(), "(empty seat)");
    if (it == playerList.end())
    {
        it  = std::find(playerList.begin(), itStart, "(empty seat)");
        if (it == itStart)
        {
            cout << "Table full" << endl;
            return -1;
        }
    }
    ...

并且

int Table::removePlayer(Player const& player)
{
    deque<Player>::iterator it = std::find(playerList.begin(), playerList.end(), "(empty seat)");
    if (it == playerList.end())
    {
        cout << "Player " << player << " not found" << endl;
        return -1;
    }
    .....