Почему создание контейнеров STL динамически считается плохой практикой? - PullRequest
7 голосов
/ 21 февраля 2011

Заголовок говорит это.

Пример плохой практики:

std::vector<Point>* FindPoints()
{
   std::vector<Point>* result = new std::vector<Point>();
   //...
   return result;
}

Что не так с ней, если я удаляю это vector позже?

Я в основном программирую наC #, поэтому эта проблема не очень понятна для меня в контексте C ++.

Ответы [ 6 ]

11 голосов
/ 21 февраля 2011

Как правило, вы этого не делаете, потому что чем меньше вы выделяете в куче, тем меньше рискует утечка памяти. :)

std::vector полезно также потому, что он автоматически управляет памятью, используемой для вектора в режиме RAII; выделив его в куче, теперь вам требуется явное освобождение (с delete result), чтобы избежать утечки памяти. Ситуация усложняется из-за исключений, которые могут изменить ваш путь возврата и пропустить любой delete, который вы указали на пути. (В C # таких проблем нет, потому что сборщик мусора периодически вызывает только недоступную память)

Если вы хотите вернуть контейнер STL, у вас есть несколько вариантов:

  • просто вернуть его по значению; теоретически вы должны понести штраф за копирование из-за временных значений, созданных в процессе возврата result, но более новые компиляторы должны иметь возможность удалить копию, используя NRVO 1 . Также могут быть реализации std::vector, которые реализуют оптимизацию копирования при записи, как это делают многие реализации std::string, но я никогда об этом не слышал.

    В компиляторах C ++ 0x семантика перемещения должна срабатывать, избегая копирования.

  • Сохраните указатель результата в интеллектуальном указателе, передающем права собственности, например std::auto_ptr (или std::unique_ptr в C ++ 0x), а также измените тип возврата вашей функции на std::auto_ptr<std::vector<Point > >; таким образом, ваш указатель всегда инкапсулируется в объекте стека, который автоматически уничтожается при выходе из функции (любым способом) и уничтожает vector, если он все еще принадлежит ей. Также совершенно ясно, кому принадлежит возвращаемый объект.
  • Сделать вектор result параметром, переданным вызывающей стороной по ссылке, и заполнить его вместо возврата нового вектора.
  • Hardcore STL опция: вы бы вместо этого предоставили свои данные в качестве итераторов; клиентский код будет затем использовать std::copy + std::back_inserter или что-то еще для хранения таких данных в любом контейнере, который он хочет. Не видел много (это может быть сложно правильно кодировать), но стоит упомянуть.
<ч />
  1. Как отметил @Steve Jessop в комментариях, NRVO работает полностью, только если возвращаемое значение используется непосредственно для инициализации переменной в вызывающем методе; в противном случае он все равно сможет исключить создание временного возвращаемого значения, но оператор присвоения переменной, которой назначено возвращаемое значение, все еще может быть вызван (подробности см. в комментариях @Steve Jessop).
6 голосов
/ 21 февраля 2011

Создание чего-либо динамически - плохая практика, если только это не действительно необходимо. Редко есть веская причина для создания контейнера динамически, поэтому обычно это не очень хорошая идея.

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

2 голосов
/ 21 февраля 2011

Создание объектов динамически в целом считается плохой практикой в ​​C ++.Что если исключение выдается из вашего кода "// ..."?Вы никогда не сможете удалить объект.Проще и безопаснее это сделать:

std::vector<Point> FindPoints()
{
  std::vector<Point> result;
  //...
  return result;
} 

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

0 голосов
/ 21 февраля 2011

Программирование - это искусство поиска хороших компромиссов.Динамически распределенная память может иметь место, конечно, и я могу даже подумать о проблемах, где хороший компромисс между сложностью кода и эффективностью достигается с помощью std::vector<std::vector<T>*>.

Однако std::vector отлично скрывает большинствоПотребности в динамически размещаемых массивах и управляемых указателях часто являются просто идеальным решением для динамически размещаемых отдельных экземпляров.Это означает, что не так часто встречаются случаи, когда неуправляемый динамически размещаемый контейнер (или динамически размещаемый независимо от того, что на самом деле) является лучшим компромиссом в C ++.

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

В вашем случае, например, я не вижу причин для использования динамического выделения;просто сделать функцию, возвращающую std :: vector, будет эффективно и безопасно.С любым приличным компилятором Оптимизация возвращаемого значения будет использоваться при назначении для вновь объявленного вектора, и если вам нужно присвоить результат существующему вектору, вы все равно можете сделать что-то вроде:

FindPoints().swap(myvector);

, который не будет выполнять какое-либо копирование данных, а лишь некоторое изменение указателя (обратите внимание, что вы не можете использовать явно более естественное myvector.swap(FindPoints()) из-за правила C ++, которое иногда раздражает и запрещает передачу временных значений в качестве неконстантных ссылок).

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

Часто вы можете найти достаточно разумные компромиссы, которые просто используют стандартные контейнеры, однако (возможно, платят некоторые дополнительные O (log N) поиски, которых вы могли бы избежать) и которые, учитывая гораздо более простой код, могут быть IMO лучшим компромиссом вбольшинство случаев.

0 голосов
/ 21 февраля 2011

Мне нравится ответ Джерри Коффина. Кроме того, если вы хотите избежать возврата копии, рассмотрите возможность передачи контейнера результатов в качестве ссылки, и иногда может понадобиться метод swap ().

void FindPoints(std::vector<Point> &points)
{
    std::vector<Point> result;
    //...
    result.swap(points);
}
0 голосов
/ 21 февраля 2011

Возможно, вы имеете в виду этот недавний вопрос: C ++: vector * args = new vector ();вызывает SIGABRT

Один вкладыш: это плохая практика, потому что это шаблон, склонный к утечкам памяти.

Вы заставляете вызывающего абонента принимать динамическое распределение и контролировать его время жизни.Из объявления неоднозначно, является ли возвращаемый указатель статическим буфером, буфером, принадлежащим другому API (или объекту), или буфером, который теперь принадлежит вызывающей стороне.Вы должны избегать этого паттерна на любом языке (включая простой C), если из названия функции не ясно, что происходит (например, strdup, malloc).

Обычный способ - сделать это:

void FindPoints(std::vector<Point>* ret) {
   std::vector<Point> result;
   //...
   ret->swap(result);
}

void caller() {
  //...
  std::vector<Point> foo;
  FindPoints(&foo);
  // foo deletes itself
}

Все объекты находятся в стеке, и все удаление выполняется компилятором.Или просто верните по значению, если вы используете компилятор C ++ 0x + STL или не против копирования.

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