Самый элегантный способ, чтобы написать этот цикл


У меня есть некоторый код, который выглядит так:

do {
    hr = pEnumMgmt->Next(1, &mgmtObjProp, &ulFetched);
    if (hr == S_FALSE) { break; }
    llCbStorage += mgmtObjProp[0].Obj.DiffArea.m_llAllocatedDiffSpace;
} while (hr != S_FALSE);

Это плохо, потому что ч == значение s_false проверяется дважды на каждой итерации.

Я мог бы написать это так:

while (true) {
    hr = pEnumMgmt->Next(1, &mgmtObjProp, &ulFetched);
    if (hr == S_FALSE) { break; }
    llCbStorage += mgmtObjProp[0].Obj.DiffArea.m_llAllocatedDiffSpace;
};

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

Наконец, я мог бы написать его с Гото, но это плохо, потому что Готоы считаются вредными. Кроме того, это становится действительно трудно проверить правильность цикла.

Как бы вы написать этот цикл и почему?



866
4
c++
задан 26 июля 2011 в 11:07 Источник Поделиться
Комментарии
4 ответа

Мои первоначальные мысли, чтобы просто потерять перерыва. Хотя они имеют свое место в Switch-операторы Case, это обычно лучше, чтобы избежать их в петли, если они используются в верхней части петли и по-настоящему помочь улучшить удобочитаемость. Даже тогда, я склонен искать альтернативы.

do {
hr = pEnumMgmt->Next(1, &mgmtObjProp, &ulFetched);
if (hr != S_FALSE) {
llCbStorage += mgmtObjProp[0].Obj.DiffArea.m_llAllocatedDiffSpace;
}
} while (hr != S_FALSE);

Кроме того, вы можете просто инициализировать час до цикла while и избежать дополнительной проверки на значение s_false каждой петли.

hr = pEnumMgmt->Next(1, &mgmtObjProp, &ulFetched);
while (hr != S_FALSE) {
llCbStorage += mgmtObjProp[0].Obj.DiffArea.m_llAllocatedDiffSpace;
hr = pEnumMgmt->Next(1, &mgmtObjProp, &ulFetched);
}

Мне не нравится следующий стиль цикл while, как он использует побочный эффект.

while ((hr = pEnumMgmt->Next(1, &mgmtObjProp, &ulFetched)) != S_FALSE) {
llCbStorage += mgmtObjProp[0].Obj.DiffArea.m_llAllocatedDiffSpace;
}

Наконец, может быть, стоит рассмотреть цикл for здесь. Мне нравится это, потому что он четко разделяет цикл от обновления llCbStorage.

for (hr = pEnumMgmt->Next(1, &mgmtObjProp, &ulFetched);
hr != S_FALSE;
hr = pEnumMgmt->Next(1, &mgmtObjProp, &ulFetched))
{
llCbStorage += mgmtObjProp[0].Obj.DiffArea.m_llAllocatedDiffSpace;
}

10
ответ дан 26 июля 2011 в 01:07 Источник Поделиться

Com-это как зомби, единственный способ быть безопасным с ним вокруг, чтобы зафиксировать ее в коробочку, из которой невозможно выбраться, потом закопать как можно глубже, просто чтобы быть уверенным.

Поскольку (в данном случае) вы перебираете ком псевдо-коллекции, чтобы сделать это на приличный c++, вы хотите, чтобы заблокировать в COM зомби-итерации внутри итератора, что-то вроде этого:

template <class T>
class COM_iterator {
HRESULT current;
unsigned long fetched;
XXX mgmtObject; // not sure of the correct type here.
T enumerator;

COM_iterator next() {
current = enumerator->Next(1, &mgmtObject, &fetched);
return *this;
}
public:
COM_iterator() : current(S_FALSE) {}

COM_iterator(T enum) : enumerator(enum) {
next();
}

COM_iterator &operator++() { return next(); }

unsigned long long operator*() {
return mgmtObjProp[0].Obj.DiffArea.m_llAllocatedDiffSpace;
}

bool operator!=(COM_iterator const &other) {
return current != other.current;
}
};

Затем мы хотим похоронить это уродство в заголовке файла. Используя его, однако, мы можем не только произвести красивее петли, но на самом деле полностью исключить саму петлю, а вместо этого использовать идиоматические алгоритм на C++:

#include <numeric>
#include "com_iterator"

long long total_bytes =
std::accumulate(COM_iterator<your_COM_type>(pEnumMgmt),
COM_iterator<your_COM_type>(),
0LL);

Как можно заметить, этот код действительно есть одна реальная проблема: это только подходит для глядя на одно конкретное свойство данных, вы смотрите. Чтобы это убрать, надо, наверное, иметь код доступа к конкретной данных (mgmtObjProp[0].Кадриров.DiffArea.m_llAllocatedDiffSpace, в данном случае) помещен в отдельную политику-как класс, который передается в качестве шаблона параметра итератор. Это позволяет разделение между кодом ком итерации, и код для доступа к внутренностям конкретного объекта вам небезразличны. Кроме того, COM_iterator может вернуться mgmtProp, и оставить его, чтобы клиентский код, чтобы выяснить, какие данные использовать из каждого элемента итерации.

Редактировать: с помощью "политики-как" экстрактор класс, вы бы начать с создания малого класса для извлечения и возврата данных:

class extract_AllocatedDiffSpace { 
XXX mgmtObject;
public:
extact_AllocatedDiffSpace(XXX init) : mgmtObject(init) {}
operator long long() { return mgmtObject[0].Obj.DiffArea.m_llAllocatedDiffSpace; }
};

... а потом добавить параметр шаблона для типа:

template <class T, class U>
class COM_iterator {
// ...
};

и переписать оператор * что-то вроде этого:

U operator *() { 
return U(mgmtProp);
}

Тогда вытяжка будет возвращен объект с помощью оператора *, а когда с std::накапливать пытался добавить, что длинный, он будет конвертировать из extract_whatever объекта на длительное использование вашего оператора длинный, который (в свою очередь) будет извлечь и вернуть в правое поле.

Вы нуждаетесь/хотите отнестись к этому с немного заботы-в extract_xxx класс должен, как правило, имеют только конструктор и один литой оператора, чтобы предотвратить его от случайного случайного использования. Если, например, у вас уже есть класс, который выступает в качестве фронт-энда для mgmtObject в другой стороны, вы, наверное, сами не хотите просто добавить оператор долго долго , чтобы он ... что делает его слишком легко для этого пересчета, который будет использоваться в других контекстах, где это не уместно.

10
ответ дан 26 июля 2011 в 04:07 Источник Поделиться

Вы можете совместить ваше состояние петли и кадровых назначений в одну строку, как это:

while ((hr = pEnumMgmt->Next(1, &mgmtObjProp, &ulFetched)) != S_FALSE) {
llCbStorage += mgmtObjProp[0].Obj.DiffArea.m_llAllocatedDiffSpace;
}

4
ответ дан 26 июля 2011 в 12:07 Источник Поделиться

while( (hr = pEnumMgmt->Next(1, &mgmtObjProp, &ulFetched)) != S_FALSE ) {
llCbStorage += mgmtObjProp[0].Obj.DiffArea.m_llAllocatedDiffSpace;
}

вышесказанное, на мой взгляд, чище способ сделать то, что вы хотите.. я не люблю так, потому что он более длинный, чем необходимо:

 for (hr = pEnumMgmt->Next(1, &mgmtObjProp, &ulFetched);
hr != S_FALSE;
hr = pEnumMgmt->Next(1, &mgmtObjProp, &ulFetched))
{
llCbStorage += mgmtObjProp[0].Obj.DiffArea.m_llAllocatedDiffSpace;
}

0
ответ дан 29 июля 2011 в 11:07 Источник Поделиться