CODEWITHSUNDEEP
c++c++11smart-pointers
Francesco Dondi
Francesco Dondi

Reputation: 1094

Being smart with smart pointers: avoiding shared_ptr overuse

I have come across code like

bool open_resource(..., shared_ptr<resource> & res)
{
   ...
   shared_ptr<resource> newResource(new resource(...));
   res = move(newResource);
   return true;
}

which is then called with 

shared_ptr<resource> res;
open_resource(..., res);

and then, as far as I saw, res is NOT used in ways that require sharing the pointer.

Of course I immediately thought of changing 

   shared_ptr<resource> newResource(new resource(...));
   res = move(newResource);

with

res = make_shared<resource>(...)

...but then I hit a roadblock. Now I can no longer advise to change the shared_ptr reference to something more basic; at least not if I want to ensure that, if the caller actually needs a shared_ptr later, the control block efficiently resides on the same allocation as the object. For this to work, it must be a shared_ptr from the beginning.

On the other side, shared_ptr is a "heavy" type; it has two counters and aliasing and all kinds of features that really seem unneeded in most calling sites. And yet if it is shared_ptr in the signature, that they have to use.

The best solution I see is to move the body of the function to a helper function, and then overload.

bool get_resource_parameters(Param1& param1,..., ParamN& paramN)
{
   ...
}

bool open_resource(..., shared_ptr<resource> & res)
{
   Param1 param1;
   ...
   ParamN paramN;
   if(!get_resource_parameters(param1,...,paramN))
       return false;

   res = make_shared<resource>(param1,...,paramN);
   return true;
}

bool open_resource(..., unique_ptr<resource> & res)
{
   Param1 param1;
   ...
   ParamN paramN;
   if(!get_resource_parameters(param1,...,paramN))
       return false;

   res = unique_ptr<resource>(new resource(param1,...,paramN));
   return true;
}

But it's really not satisfying.

Does anyone see a better, more C++ solution?

Edit

Yes, the C++ way would be to return the pointer rather than a bool (and check for null). I cannot overload for shared_ptr in this case, but I can then assign the unique_ptr temporary returned to a shared_ptr varaible, and the appropriate constructor will convert it.

However, this way I lose the single allocation of make_shared. can I save it?

Upvotes: 3

Views: 2289

Answers (2)

Jarod42
Jarod42

Reputation: 218268

To allow unique_ptr/shared_ptr, you may use template:

// Dispatcher for make_unique/make_shared
template <template <typename...> class Ptr, typename T>
struct make_helper;

template <typename T>
struct make_helper<std::unique_ptr, T>
{
    template <typename ...Ts>
    std::unique_ptr<T> operator() (Ts&&... args) const {
        return std::make_unique<T>(std::forward<Ts>(args)...);
    }
};

template <typename T>
struct make_helper<std::shared_ptr, T>
{
    template <typename ...Ts>
    std::shared_ptr<T> operator() (Ts&&... args) const {
        return std::make_shared<T>(std::forward<Ts>(args)...);
    }
};

template <template <typename...> class Ptr, typename T, typename ... Ts>
auto make(Ts&&... args)
{
    return make_helper<Ptr, T>{}(std::forward<Ts>(args)...);
}

And then

bool get_resource_parameters(Param1& param1,..., ParamN& paramN)
{
    //...
}

template <template <typename...> class Ptr>
Ptr<resource> open_resource(...)
{
   Param1 param1;
   ...
   ParamN paramN;
   if(!get_resource_parameters(param1, ..., paramN))
       return nullptr;

   return = make<Ptr, resource>(param1, ..., paramN);
}

And check for nullptr instead of split bool and smart_pointer.

Upvotes: 4

Maxim Egorushkin
Maxim Egorushkin

Reputation: 136515

std::shared_ptr has a converting constructor from std::unique_ptr. Why don't you make the function return a std::unique_ptr by value:

unique_ptr<resource> open_resource(...);

This also serves as documentation that this is a factory function that transfers the ownership of the resource to the caller.

And let the caller decide how they want it:

auto x = open_resource(...);
// or
std::shared_ptr<resource> x{open_resource(...)};

Upvotes: 12

Related Questions