Обратная связь потребности на двух функциях членства класса Таблицы в C++

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;
}

Хотел бы некоторую обратную связь на этих двух функциях членства класса Таблицы для моделирования покера Техас Холдем. Любой информационный синтаксис, эффективность или даже общие методы очень ценились бы.

1
задан user83 2 June 2010 в 14:58
поделиться

4 ответа

Ваш первый цикл while в addPlayer () разыменовывает итератор, действительность которого не проверялась. Если передается значение позиции, превышающее количество элементов в контейнере, скорее всего, произойдет сбой. Это может контролироваться вызывающим, но гораздо лучше контролировать его в точке отсчета.

2
ответ дан 3 September 2019 в 00:08
поделиться

удаление может быть выполнено в цикле 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;

Я считаю, что это немного чище, и лично я большой поклонник комментариев.

0
ответ дан 3 September 2019 в 00:08
поделиться

1) Если вы не собираетесь изменять параметр в методе, переходите по ссылке на константу:

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

Это обеспечивает скрытые преимущества последнего, но также вводит концепцию корректности констант.

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;
    }
    .....
0
ответ дан 3 September 2019 в 00:08
поделиться
  • Сделайте отступ в коде правильно, это значительно упрощает чтение и понимание позже. Если у вас нет опыта, подумайте о том, чтобы принять руководство по стилю (например, Руководство по стилю Google C ++ ).
  • Убедившись, что разыменование итератора игрока на "(пустое место)" вызывает сомнения, вы можете рассмотреть несколько альтернатив:
    • Ведите отдельный список пустых стульев и размещайте их на AddPlayer () .
    • Позвольте полиморфизму летать и используйте класс EmptySeatPlayer .
    • Разрешить сохранение нулевых игроков непосредственно в tableList .
  • Мне непонятно, зачем AddPlayer нужен параметр position , если он просто выделяет следующее доступное место (пока не найдет его). Возможно, полностью удалите параметр и позвольте таблице выяснить это.
  • В конечном итоге вы, вероятно, захотите отделить игровой текст от бизнес-логики.
  • Вероятно, вам не следует использовать позицию напрямую, вы должны оперировать игроками и столом. Можно было бы считать это деталью класса, которая не должна предоставляться функциями.
  • Вместо while (* it! = Player) и проверки завершения на каждой итерации используйте std :: find .
  • Также вы часто выполняете «передачу по значению», рекомендуется передавать const Player & , чтобы избежать ненужных копий.
0
ответ дан 3 September 2019 в 00:08
поделиться
Другие вопросы по тегам:

Похожие вопросы: