Is the following a reasonable and efficient approach with regard to creating Stuff and giving Foo ownership of it?
class Foo
{
explicit Foo(const std::shared_ptr<Stuff>& myStuff)
: m_myStuff(myStuff)
{
}
...
private:
const std::shared_ptr<Stuff> m_myStuff;
}
std::shared_ptr<Stuff> foosStuff(new Stuff());
Foo f(foosStuff);
Since you are interested in efficiency I'd like to make two points:
shared_ptr<> is one of many standard library types where a move construction is cheaper than a copy construction. Copy constructing shared_ptr is slower since copying requires the reference counters to be incremented atomically whereas moving a shared_ptr doesn't require touching the referenced data or counters at all. From the article "Want Speed? Pass by value!" by Dave Abrahams, one can learn that in certain situations it is actually beneficial to take a function parameter by value. This is one of these cases:
class Foo
{
explicit Foo(std::shared_ptr<Stuff> myStuff)
: m_myStuff(move(myStuff))
{}
...
private:
std::shared_ptr<Stuff> m_myStuff;
};
Now you can write
Foo f(std::make_shared<Stuff>());
where the argument is a temporary and no shared_ptr is ever copied (just moved once or twice).
The use of std::make_shared here has the advantage that only one allocation is done. In your case you allocated the Stuff object by yourself and the shared_ptr constructor had to allocate the reference counters and deleter dynamically as well. make_shared does it all for you with just a single allocation.
Yes, that's perfectly reasonable. This way the work involved in managing the shared pointer will only need to be done once, rather than twice if you passed it by value. You could also consider using make_shared to avoid a copy construction.
std::shared_ptr<Stuff> foosStuff(std::make_shared<Stuff>());
The only improvement you could make is if Foo is to be the sole owner (ie. you're not going to keep foosStuff around after creating Foo) then you could switch to using a std::unique_ptr or boost::scoped_ptr (which is the pre-C++11 equivalent), which would have less overhead.
scoped_ptr, it's not part of C++? Maybe you mean std::unique_ptr? And why the custom deleter?
boost::scope_ptr. The deleter bit may be an outdated reference to an earlier (stealth) edit?It might be more efficient to have a make_foo helper:
Foo make_foo() { return Foo(std::make_shared<Stuff>()); }
Now you can say auto f = make_foo();. Or at the very least use the make_shared invocation yourself, since the resulting shared_ptr may be more efficient than the one constructed from a new expression. And if Stuff actually takes constructor arguments, a private auxiliary constructor might be suitable:
struct Foo
{
template <typename ...Args>
static Foo make(Args &&... args)
{
return Foo(direct_construct(), std::forward<Args>(args)...);
};
private:
struct direct_construct{};
template <typeaname ...Args>
Foo(direct_construct, Args &&... args)
: m_myStuff(std::make_shared<Stuff>(std::forward<Args>(args)...)) // #1
{ }
};
You can either wrap Foo::make into the above make_foo, or use it directly:
auto f = Foo::make(true, 'x', Blue);
That said, unless you're really sharing ownership, a std::unique_ptr<Stuff> sounds like more preferable approach: It is both conceptually much simpler and also somewhat more efficient. In that case you'd say m_myStuff(new Stuff(std::forward<Args>(args)...)) in the line marked #1.
Foodoesn't take ownership. The point ofshared_ptris to share ownership.std::shared_ptr<Stuff> foosStuff(new Stuff());