1 
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 类的这两个成员函数有一些反馈。任何信息语法、效率甚至常见做法都将不胜感激。

4

4 回答 4

2

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

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

删除可以在 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;

我觉得这有点干净，我个人是评论的忠实粉丝。

于 2010-06-02T04:05:28.447 回答
0

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;
    }
    .....
于 2010-06-02T06:05:36.540 回答