2015-01-13 2 views
2

Учитывая следующий пример кода:Должен ли я инициализировать shared_ptr внутри или вне класса конструктора?

#include <memory> 

class Foo { 
public: 
    Foo(std::shared_ptr<int> p); 
private: 
    std::shared_ptr<int> ptr; 
}; 

Foo::Foo(std::shared_ptr<int> p) : ptr(std::move(p)) { 
} 

class Bar { 
public: 
    Bar(int &p); 
private: 
    std::shared_ptr<int> ptr; 
}; 

Bar::Bar(int &p) : ptr(std::make_shared<int>(p)) { 
} 

int main() { 
    Foo foo(std::make_shared<int>(int(256))); 
    Bar bar(*new int(512)); 

    return 0; 
} 

Оба Foo и Bar работать правильно. Однако существуют ли какие-либо различия между созданием shared_ptr при вызове конструктора, а затем переносом права собственности на std :: move и просто передачей ссылки на объект и делегированием создания shared_ptr в конструктор класса?

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

Какой я должен использовать и почему?

+1

Я предпочитаю прямую инициализацию с помощью 'std :: make_shared'. Если «новые» выбросы имеют определенную утечку памяти. –

+2

Код утечки памяти, две вещи совершенно разные –

ответ

6

Foo is correct.

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

EDIT: информация об утечке памяти.

Разбирая эту строку:

Bar bar(*new int(512)); 

результаты этих операций:

  1. вызов новым Int (512), что приводит к вызову оператора нового, возвращая указатель на междунар в куче (выделение памяти).
  2. разыменовать указатель, чтобы предоставить ссылку на const для конструктора бара
  3. Затем панель создает свой shared_ptr с одним возвратом make_shared (эта часть является эффективной). Этот int shared_ptr инициализируется копией int, переданной по ссылке.
  4. тогда функция возвращает, но поскольку никакая переменная не записала указатель, возвращенный из нового, ничто не может освободить память. Каждое новое должно быть отражено удалением, чтобы уничтожить объект и освободить его память.
  5. поэтому утечку памяти
+0

Можете ли вы аргументировать лучше «утечку памяти» и «освобождение от небезопасности»? –

+0

Я отредактирую ответ –

+0

Это прекрасно, но проблема не в баре, а в том, как он использовался. Но вы заявляете, что abnomination - это Bar, в то время как реальная проблема - * новая (что делает указатель возвращен новым недоступным и, следовательно, неуязвимым) –

3

Это зависит от того, чего вы хотите достичь.

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

Если вы хотите получить совместное владение существующим объектом (это более распространенный случай, действительно), у вас нет выбора, и вам нужно использовать первый способ.

Если ни то, ни другое не применимо, вам, вероятно, не обязательно нужен shared_ptr.

1

Второй способ неправильный; он теряет память. С

Bar::Bar(int &p) : ptr(std::make_shared<int>(p)) { 
} 

.... 

Bar bar(*new int(512)); 

shared_ptr сделаны std::make_shared принимает значение p (которое 512), чтобы построить новый общий указатель, который отвечает за новую часть памяти; он не несет ответственности за адрес памяти, где находится p.Этот кусок - тот, который вы выделили в main, - затем теряется. Этот конкретный фрагмент кода будет работать с

     // +---------------------- building a shared_ptr directly 
         // v    v----- from p's address 
Bar::Bar(int &p) : ptr(std::shared_ptr<int>(&p)) { 

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

Вы могли бы более здраво написать

Bar::Bar(int *p) : ptr(p) { 
} 

.... 

Bar bar(new int(512)); 

На самом деле вы могли бы дать Foo второй конструктор, который делает это, если вы хотите. Это немного аргумент, как ясно, что сигнатура функции делает это тем, что указатель должен быть для объекта, выделенного кучей, но std::shared_ptr предлагает то же самое, поэтому существует прецедент. Это зависит от того, что делает ваш класс, это хорошая идея.

+0

Почему бы пользователю сделать выделение кучи в первую очередь? Вы можете так же легко принять аргумент, указав не принадлежащие (rvalue), и скопируйте (переместите) его в 'shared_ptr' в случае« Bar ». – ComicSansMS

+0

Одна из причин, по которой вы, возможно, захотите, чтобы это было связано с тем, что вам нужно взять собственность от 'std :: unique_ptr' или обработать устаревший код, который использует 'std :: auto_ptr' или' boost :: scoped_ptr'. Иногда вы не можете свободно выбирать свои библиотеки, а затем что-то вроде этого может сократить шаблонный код. Вероятно, это не средство, которое вы часто используете, но может иметь смысл иметь его на месте. Иногда. – Wintermute

+0

Хорошая точка. В этом случае, однако, я бы утвердил, что подход «Foo» является предпочтительным, что делает его более очевидным, что собственность передается конструктору объекта. Тем не менее, хороший анализ того, почему был нарушен исходный код для «Бар». – ComicSansMS

0

Оба могут работать правильно, но способ использования их в main несовместим.

Когда я вижу конструктор, использующий ссылку (например, Bar(int& p)), я ожидаю, что Bar проведет ссылку. Когда я вижу Bar(const int& p), я ожидаю, что он сохранит копию. Когда я вижу RValue реф (не универсальны, как Bar(int&& p) Я ожидаю, что р не «выжить его содержание» после пройдена. (Ну ... для него это не имеет смысла, что ...).

В любом случае p держит int, не pointer, и что make_shared ожидает являются параметр пересылается в Int конструктор (то есть ... ИНТ, не Int *).

Ваш главный поэтому должны быть

Foo foo(std::make_shared<int>(int(256))); 
Bar bar(512); 

Это приведет к тому, что бар удерживает динамически распределенную скопированную копию значения 512.

Если вы делаете Bar bar(*new int(512)), вы делаете бар для хранения копии вашего «нового int», указатель которого отбрасывается, и, следовательно, сам int просочился.

В общем, такие выражения, как *new something должно звучать как «нет, нет, нет, нет ...»

Но вы Bar конструктор имеет проблему: чтобы быть в состоянии взять константу или выражение возвращаемое значение, оно должно взять const int&, не int&

0

Если ваше намерение принять исключительное право собственности на куче выделяется объект, я предлагаю вам принять std::unique_ptr. Это документы о намерении ясно, и вы можете создать std::shared_ptr из std::unique_ptr внутри, если вам нужно:

#include <memory> 

class Foo { 
public: 
    Foo(std::unique_ptr<int> p); 
private: 
    std::shared_ptr<int> ptr; 
}; 

Foo::Foo(std::unique_ptr<int> p) : ptr(std::move(p)) { 
} 

int main() { 
    Foo foo(std::make_unique<int>(512)); 
} 

Не делайте Bar, это чревато ошибками и не в состоянии описать свое намерение (если я понимаю ваше намерение правильно).

0

Предполагая, что вы использовали int как пример, и вместо этого существует реальный ресурс, это зависит от того, чего вы хотите достичь.

Первый представляет собой типичную инъекцию зависимостей, где объект вводится через конструктор.Хорошая сторона заключается в том, что он может упростить модульное тестирование.

Второй случай просто создает объект в конструкторе и инициализирует его с использованием значений, переданных через конструктор.


Кстати, позаботьтесь о том, как вы инициализируетесь. Это:

Bar bar(*new int(512)); 

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

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