Polymorphic allocation of shared_ptr member variable

Question

I have recently attempted to learn how to use std::shared_ptr. When modifying my existing code I've found myself confused when allocating with member variables (outside of an initialisation list).

My old code:

Class A {

Base* member_var;

A() {
    this->member_var = new Derived();
}

};

My new code:

Class A {

std::shared_ptr<Base> member_var;

A() {
    //Note the shared_ptr is given type Derived, matching the object.
    this->member_var = std::shared_ptr<Derived> (new Derived());
}

};

Is this correct? Or is this perhaps more correct?:

Class A {

std::shared_ptr<Base> member_var;

A() {
    //Note the shared_ptr is of type Base, matching the member type.
    this->member_var = std::shared_ptr<Base> (new Derived());
}

};

What is the difference between these two statements.

Worryingly, I can't seem to find any code examples of what I'm trying to do here. Is my approach to using std::shared_ptr wrong?

---

EDIT: Thanks to everyone for their help. I think I caused some confusion. For the sake of future readability, I'll expand on why I've chosen to take this approach. I chose to illustrate my question with a simple code example. In my actual problem, I don't use a Class A, I actually use a struct. The sole purpose of this struct, is to help me neatly hold on to a number of instances of various different objects. I frequently end up passing (by reference) each object individually, not the struct itself, as argument to functions. Furthermore, when I do give the entire struct as argument, I tend to pass-by-value this struct. Hence my interest in making these shared_ptr, not unique_ptr. I've been debating changing everything and encapsulating all these in a Class, like Class A in my example, and passing said instance of class pass-by-reference the object instance. In this instance, I agree with everyone who has commented, and unique_ptr seems more appropriate.

Of course there's no fundamental difference between a struct and a class. I just have a tendency to pass instances of structs by value (if all they contain are pointers), and instances of classes by reference. Perhaps that's a personal quirk of mine?

Regarding the use of polymorphism in the first place, there are two possible derived classes here, one of which is chosen at runtime. I wish to handle the base class, which is for all intents and purposes, an abstract class.

I'm still surprised this is not a more common situation. Perhaps the above paragraphs have highlighted further bad practice. In which case I would be grateful to hear about it.

Answer 1

I'd write it this way:

A() : member_var(std::make_shared<Derived>()) { }

In Modern C++ you should avoid using new whenever possible. If you cannot initialize your member_var in the initialization list, then do:

A()
{
    // ...
    member_var = std::make_shared<Derived>();
}

As mentioned by KerrekSB in the comments, I would also suggest considering whether you do indeed need shared ownership, or wheter a unique_ptr wouldn't be sufficient.

If that is the case, you should use a unique_ptr instead: in general, it is a good thing to express minimal requirements - this buys you performance and a better self-documenting code at the same time.

Unfortunately, in C++11 you sill have to resort to new to create a unique_ptr that points to a newly created object:

A() : member_var(new Derived()) { }

Or, if you cannot use initialization lists:

member_var.reset(new Derived());

On the other hand in C++14, which offers std::make_unique<>, I would rather write:

A() : member_var(std::make_unique<Derived>()) { }

Or, if you cannot use initialization lists:

member_var = std::make_unique<Derived>();