Является ли этот код злоупотреблением STL find_if? - PullRequest
4 голосов
/ 26 августа 2008

Допустим, у меня есть список имен серверов, хранящихся в векторе, и я хотел бы связываться с ними по одному, пока один из них не ответит успешно. Я думал об использовании алгоритма STL find_if следующим образом:

find_if(serverNames.begin(), serverNames.end(), ContactServer());

Где ContactServer - объект функции предиката.
С одной стороны, существует проблема, поскольку предикат не всегда будет возвращать один и тот же результат для одного и того же имени сервера (из-за простоя сервера, проблем с сетью и т. Д.). Однако один и тот же результат будет возвращен независимо от того, какая копия предиката используется (, т. Е. предикат не имеет реального состояния), поэтому исходная проблема с предикатами с сохранением состояния в данном случае не имеет значения. 1007 *

Что ты скажешь?

Ответы [ 8 ]

4 голосов
/ 26 августа 2008

Я думаю, я бы пошел на это.

Единственное, о чем я бы беспокоился, это его удобочитаемость (и, следовательно, удобство обслуживания) Для меня это звучит как «Найти первый сервер, с которым я могу связаться», что имеет смысл.

Возможно, вы захотите переименовать ContactServer, чтобы указать, что это предикат; CanContactServer? (Но тогда люди будут жаловаться на скрытые побочные эффекты. Хм ...)

4 голосов
/ 22 сентября 2008

Это как раз то, для чего предназначены алгоритмы STL. Это совсем не оскорбление. Кроме того, это очень читабельно. Перенаправьте на ноль любого, кто скажет вам иначе.

1 голос
/ 25 мая 2012

На мой взгляд, использование std::find_if немного вводит в заблуждение. Когда я читаю этот фрагмент кода, я не ожидаю каких-либо побочных эффектов, я просто ожидаю, что будет найдено имя сервера. Тот факт, что результат find_if отбрасывается, также заставляет меня задуматься, действительно ли код верен. Возможно, другое имя предиката прояснит намерение, но я думаю, что проблема более фундаментальная.

Для большинства людей find_if - это алгоритм , запрашивающий , а не модифицирующий алгоритм . Даже если вы на самом деле не изменяете значения, повторяемые на основе, вы изменяете глобальное состояние вашего приложения (в этом случае вы даже можете изменять состояние удаленных серверов).

В таком случае я бы, вероятно, придерживался ручного цикла, особенно теперь, когда в C ++ 11 введены циклы на основе диапазона:

for (std::string const & name : serverNames)
{
    if (ContactServer(name)) break;
}

Другим решением было бы заключить это в функцию с именем, передающим более ясное намерение, например apply_until или что-то в этом роде:

template <typename InputIterator, typename Function>
void apply_until(InputIterator first, InputIterator last, Function f)
{
    std::find_if(first, last, f);
    // or
    // while (first != last)
    // {
    //     if (f(*first)) break;
    //
    //     ++first;
    // }
}
}

Но, возможно, я чрезмерно пуристичен:)!

0 голосов
/ 19 сентября 2008

find_if представляется правильным выбором здесь. В этой ситуации предикат не является состоящим

0 голосов
/ 19 сентября 2008

std :: for_each может быть лучшим кандидатом для этого.

1) После копирования в одну и ту же функцию объект используется для каждого элемента, а после обработки всех элементов копия потенциально обновленного объекта функции возвращается пользователю.

2) На мой взгляд, это также улучшило бы удобочитаемость звонка.

Объект функции и вызов for_each будут выглядеть примерно так:


struct AttemptServerContact {
  bool        server_contacted;
  std::string active_server;    // name of server contacted

  AttemptServerContact() : server_contacted(false) {}

  void operator()(Server& s) {
    if (!server_contacted) {
      //attempt to contact s
      //if successful, set server_contacted and active_server
    }
  }
};

AttemptServerContact func;
func = std::for_each(serverNames.begin(), serverNames.end(), func);
//func.server_contacted and func.active_server contain server information.
0 голосов
/ 02 сентября 2008

В следующей версии стандарта C ++ я не смог найти каких-либо явных ограничений в отношении того факта, что предикат всегда должен возвращать одно и то же значение для одного и того же ввода. Я посмотрел в раздел 25 (пункты 7-10).

Методы, возвращающие значения, которые могут меняться от одного вызова к другому, как в вашем случае, должны быть volatile (начиная с 7.1.6.1/11: «volatile - это подсказка реализации, чтобы избежать агрессивной оптимизации с участием объекта, поскольку значение объект может быть изменен средствами, не обнаруживаемыми реализацией ").

Предикаты "не должны применять какие-либо непостоянные функции через разыменованные итераторы" (параграфы 7 и 8). Я предполагаю, что это означает, что они не обязаны использовать энергонезависимые методы, и что ваш сценарий использования, таким образом, соответствует стандарту.

Если бы формулировка была "предикаты должны применять константные функции ..." или что-то в этом роде, то я бы пришел к выводу, что функции "const volatile" были не в порядке. Но это не так.

0 голосов
/ 26 августа 2008

Однако один и тот же результат будет возвращен независимо от того, какая копия предиката используется (т. Е. Предикат не имеет реального состояния), поэтому исходная проблема с предикатами с сохранением состояния в этом случае не имеет значения.

Так в чем же проблема? Функциональные объекты не обязательно должны быть с состоянием. На самом деле, в таких ситуациях лучше использовать функциональные объекты вместо указателей функций, потому что компиляторы лучше их вставляют. В вашем случае создание экземпляра и вызов объекта функции могут вообще не иметь затрат, поскольку find_if является шаблоном функции, а компилятор сгенерирует собственную версию для вашего функтора.

С другой стороны, использование указателя на функцию приведет к косвенному направлению.

0 голосов
/ 26 августа 2008

Разве не для этого find_if?

Обратите внимание, что он найдет все серверы, если вы перебираете итератор - но вы не собираетесь этого делать (в соответствии с OP).

Добро пожаловать на сайт PullRequest, где вы можете задавать вопросы и получать ответы от других членов сообщества.
...