2012-01-10 6 views
0

Парень overflower сказал, что это сделало его глаза кровоточат:лучший стиль кодирования для итераторов

for (std::vector<Agent*>::const_iterator iter = agents.begin(); iter != agents.end(); ++iter) 
{ 
    delete *iter; 
} 

Что аккуратнее способ написания его? Я предполагаю, что я мог бы набрать тип итератора, но в некоторых классах это означало бы огромный блок typedef наверху, который, по моему мнению, выглядит ужасно.

+0

Этот вопрос не по теме: разместите его на codereview – akappa

+0

Вы можете по крайней мере разбить 3 компонента for на отдельные строки –

+0

Использовать вектор интеллектуальных указателей. – kennytm

ответ

0
std::vector<Agent*>::const_iterator iter = agents.begin(); 


while(iter != agents.end()) 
{ 
    delete * iter; 
    iter++; 
} 
0

Использование Boost Foreach с #define, чтобы сделать его еще красивее.

#include <boost/foreach.hpp> 
#define foreach BOOST_FOREACH 

... 

foreach(Agent* p, agents) 
{ 
    delete p; 
} 
0

Ну, мои глаза кровоточат, потому что эта линия превышает 80 колонок :)

Попробуйте это:

std::vector<Agent*>::const_iterator iter; 
for (iter = agents.begin(); iter != agents.end(); ++iter) 
{ 
    delete *iter; 
} 
+0

Это оставляет 'iter' зависанием в охватывающей области, тогда как определение его в цикле' for' не делает. – arne

1

ли увеличить то, что вы можете рассмотреть? В Еогеаспе такого типичный "для каждого" цикла, вы можете пользоваться усилением в:

#include <boost/foreach.hpp> 

BOOST_FOREACH(Agent* a, agents) { 
    delete a; 
} 

http://www.boost.org/doc/libs/1_48_0/doc/html/foreach.html

2

Я бы std::vector<Agent*> определения типа во.

+0

Это на самом деле очень хороший момент, это избавляет меня от необходимости typedef :: size_type и :: iterator и :: const_iterator все отдельно – SirYakalot

2

В C++ 11 вы можете сделать range based for петли

map<string, string> address_book; 
for (auto address_entry : address_book) 
{ 
    cout << address_entry.first << " < " << address_entry.second << ">" << endl; 
} 
2
#include <algorithm> 

void delete_(Agent* agent) { 
    delete agent; 
} 
... 

std::for_each(agents.begin(), agents.end(), delete_); 
0

C++ 2011 ваш друг:

std::for_each(agents.begin(), agents.end(), 
         [](Agent* agent){ delete agent; }); 
3

Вы получили ряд ответов уже, но Я полагаю, что проблема намного ... глубже, чем они предполагают. Большинство из них смотрят в основном (или исключительно) на синтаксис, который вы используете, который, как я думаю, в целом является наименьшей из проблем здесь. Они лечат симптом, а не болезнь.

Для цикла for, который вы опубликовали, чтобы иметь смысл, у вас есть коллекция указателей на объекты, выделенные new. Более того, тот факт, что вы удаляете все объекты в контейнере, подразумевает, что вы в основном связываете права собственности на объекты с этим контейнером.

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

Это вызывает серьезный вопрос: почему вы храните указатели на объекты в первую очередь? Возможно, объекты дороги для копирования, и вы опасаетесь, что когда вектор расширит свое выделение, копирование объектов будет слишком медленным. Скорее всего, в этом случае вы просто ошибаетесь. Хорошо известно, что vector расширяет свое распределение экспоненциально, так что вставка амортизировала постоянную сложность. То, что не так хорошо известно или очевидно, состоит в том, что тот же экспоненциальный рост также означает, что среднее число копий существующих объектов асимптотически приближается к константе. В типичной реализации среднее количество копий будет где-то между 2 и 3. Таким образом, шансы довольно хороши, что вы можете просто создать вектор объектов, и ваша эффективность останется вполне адекватной. В этом случае ваша петля превращается в (не более) agents.clear() (и шансы довольно хорошие, даже если это действительно не понадобится).

Если вы находитесь в одной из относительно редких ситуаций, когда это копирование действительно является проблемой, вы можете захотеть (например) определить agents как vector<shared_ptr<agent> > вместо использования необработанных указателей. Здесь снова не требуется удаление объектов-указателей, так как shared_ptr будет обрабатывать это автоматически, когда количество ссылок на объекты достигнет 0.

Для C++ 11 вы можете (часто) выполнять то же самое, поддерживая семантику перемещения вместо копирования. Предполагая, что вы можете рассчитывать на поддержку во всех своих компиляторах, это может еще больше повысить простоту и эффективность.

Это, вероятно, не исчерпывающий список возможностей, а некоторые из других могут привести к слегка различным излечениям. Точка остается прежней: выясните, что действительно неправильно, и исправьте это.

Повторяю, однако: прямо сейчас вы смотрите на симптом. Вам нужно исправить болезнь, которая заставит симптом исчезнуть.

Редактировать: Как обычно, кто-то, кажется, неправильно понял (или не понял), как работают векторы. Для аргумента, я буду использовать его пример из 64K элементов. Чтобы сохранить математику как можно проще, я также предполагаю, что вектор полностью заполнен и что эта реализация точно удваивает размер вектора при необходимости изменения размера.

В этом случае 32K элементов никогда не копировались. Еще один 16K был скопирован один раз. Еще 8K были скопированы дважды. Еще 4K были скопированы три раза, 2K четыре раза, 1K пять раз, 512 шесть раз, 256 семь, 128 восемь, 64 девять, 32 десять, 16 одиннадцать, 8 двенадцать, четыренадцать и двадцать четыре раза и 1 пятнадцать раз (хотя, конечно, на самом деле реальная реализация, вероятно, начнется с по крайней мере 8 или 10 элементов, хотя это мало чем отличается). Разделив эту сумму на 64К дает нам среднее количество раз элементы были скопированы:

Итак, что мы получаем:

32K* 0 
+16K* 1 
+ 8k* 2 
+ 4K* 3 
+ 2K* 4 
+ 1K* 5 
+512* 6 
+256* 7 
+128* 8 
+ 64* 9 
+ 32* 10 
+ 16* 11 
+ 8* 12 
+ 4* 13 
+ 2* 14 
+ 1* 15 
= 65519 

Разделив что 65536 дает среднее число раз элемент в векторе был скопирован - 0.999741. Если мы добавим еще один элемент, мы будем копировать каждый из этих элементов еще раз и иметь только один элемент, который был скопирован 0 раз. Это дает нам в среднем 1,99971.

Я сомневаюсь, что для получения более высоких и низких оценок от них: 1 и 2, требуется много фантазии.

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

+0

Я вообще не согласен с тем, что вы написали в этом посте. Это субъективно, поэтому я не буду его понижать. Фактическая ошибка: при типичной реализации размер вектора обычно * не более * удваивается, поэтому количество копий равно * не менее * O (log n). 64k элементов ~ = 16 копий. – zvrba

+0

@zvrba: вы, конечно, приветствуете свое мнение - хотя это неправильно! :-) Что же касается ограничения количества копий, то, похоже, вы упустили тот факт, что я говорил о среднем - или, по крайней мере, о том, что это подразумевало. См. Отредактированный ответ для дальнейшего объяснения. –

+0

+1 Я бы предложил 'boost :: ptr_vector', или' boost :: poly_collection :: base_container' (a la http://bannalia.blogspot.nl/2014/05/fast-polymorphic-collections.html). То есть без дополнительной информации о конкретном случае использования – sehe

0

Я подозреваю, что он жаловался на явное выписывание цикла for для диапазона STL. Существуют альтернативы, например std::for_each от <algorithms>. С более новыми компиляторами вы также можете использовать auto и лямбда-выражения, чтобы написать это более кратко.

Смежные вопросы