Using Shared Pointers in Public Interfaces
We have a fairly standard tree API using shared pointers that looks something like this (implementation omitted for brevity):
class node;
using node_ptr = std::shared_ptr<node>;
class node : public std::enable_shared_from_this<node> {
std::weak_ptr<node> parent;
std::vector<node_ptr> children;
public:
virtual ~node() = default;
virtual void do_something() = 0;
void add_child(node_ptr new_child);
void remove_child(node_ptr child);
node_ptr get_parent();
const std::vector<node_ptr>& get_children();
};
class derived_node : public node {
derived_node() = default;
public:
virtual void do_something() override;
static node_ptr create(/* args... */);
};
// More derived node types...
This works great and prevents the nodes from leaking as you might imagine. However, I have read about other answers on SO that use std::shared_ptr
public API as considered bad style and should be avoided.
Obviously it was risking based on opinions, so a few specific questions to keep this question from getting closed :-)
-
Are there any known usage issues
shared_ptr
in such interfaces that we have so far managed to avoid? -
If so, is there a commonly used (I hesitate to say "idiomatic") alternative formulation that avoids the above errors but still allows simple memory management for users?
Thank.
source to share
Not a bad style, it depends on your goals and assumptions.
Several projects I worked on with tight constraints required us to avoid shared_ptrs because we wanted to manage our own memory. Therefore, there is no use of third party libraries that require the use of shared_ptrs.
Another reason you might want to avoid shared_ptrs is that it is somewhat stubborn. Some projects will wrap everything around and just pretend it has a GC (Urg!) Language. Other projects will treat shared_ptrs with a little more restraint and only use shared_ptrs when it comes to things that have common property.
Most of the third party APIs (of course not all) that I have worked with work by the principle, if you highlight it, you destroy it. As long as you are very clear about resource ownership, this doesn't cause too many problems.
std::shared_ptr
- manage property,
so prefer for function print_tree
void print_tree(const node& root); // no owner ship transfer
than
void print_tree(const std::shared_ptr<node>& root);
The later version is required shared_ptr
, so the object may need to be built shared_ptr
. (while fetching an object from shared_ptr
is a simple getter)
Now, for your getters, you mainly have a choice between
-
share_ptr
if you want to share ownership with a user -
weak_ptr
, a secure link to the internal - pointer / link, unsafe internal link.
Safe and unsafe I mean if the object is destroyed, you can check it with weak_ptr
, but not with a simple pointer. Security has some overhead, so there is a trade-off.
If the accessors are for local use only and don't support referencing, a pointer / link might be a good option. As an example, are std::vector::iterator
unusable after destruction of a vector, so there is good scope for local use, but can be dangerous to keep an iterator as a reference (but possible).
Do you expect / allow this user to keep a reference to the node, but allow destruction of the root (or parent)? what should happen to this node for the user?
For void add_child(node_ptr new_child);
, you are clearly taking responsibility. You can hide shared_ptr
if node build it's child itself, something like
template <typename NodeType, typename...Ts>
std::weak_ptr<NodeType> add_child(Ts&&... args)
{
auto n = std::make_shared<NodeType>(std::forward<Ts>(args)...);
// or
// auto n = NodeType::create(std::forward<Ts>(args)...);
// Stuff as set parent and add to children vector
return n;
}
source to share
One of the reasons this might be considered bad style may be the additional overhead associated with reference counting, which is often (never say never) that is actually unnecessary in external APIs as they often fall into one of two categories:
- The API that receives the pointer acts on it and returns it - the original pointer will usually perform better since the function doesn't need to manipulate the pointer itself in any way.
- An API that manages a pointer, for example in your case
std::unique_ptr
, is generally better suited and has 0 overhead.
source to share