Can I use shared_ptrs in this example?

Question

I have a simple event handling system that is giving me issues. To use it I inherit from the class EventHandler. The constructor then registers each object on construction.

Here is EventHandler's constructor:

EventHandler::EventHandler()
{
   EventDispatcher::getInstance().registerListener(this);
}

This calls EventDispatcher's registerListener() member function which stores this in a vector.

void EventDispatcher::registerListener(EventHandler* listener) 
{
   mListenerList.push_back(listener);
}

Where is mLisernerList looks like:

vector<EventHandler*> mListenerList;

The EventDispatcher simply calls the sendEvent() on each element of the vector to notify it of the event.

Let me give an example to demonstrate my issue. Lets say my class Buttons inherits from EventHandler. I will create button objects on the heap and then place smart pointers to all my buttons in a single vector.

vector<unique_ptr<Buttons>> mButtons;   
mButtons.push_back(unique_ptr<Buttons>(new Button()));

What happens is I end up with a vector of unique_ptrs in mButtons and a vector of raw pointers in mListenerList pointing to the same dynamically allocated Button objects. I do not want smart and raw pointers pointing to the same object.

Ideally, I would like a vector of shared_ptrs in mButtons and a vector of weak_ptrs in mListenerList pointing to the dynamically allocated Button objects while allowing EventHandler to register every object on creation. Is this possible?

It is possible. You might want to look into `std::enable_shared_from_this` — OmnipotentEntity, May 06 '14 at 16:57
@OmnipotentEntity I realize I will need to use enable_shared_from_this to avoid issues with this. While that may be the more subtle issue I am just stumped on keeping the current behavior of EventHandler, but expanding it to share a pointer. — user870130, May 06 '14 at 17:02
What is wrong with smart and raw pointers pointing to the same object? As long as the smart pointers do the "owning" and the raw pointers do not. — Chris Drew, May 06 '14 at 18:21
@Chris Drew I following this advice, "If you want to get the full beneﬁt of smart pointers, your code should avoid using raw pointers to refer to the same objects; otherwise it is too easy to have problems with dangling pointers or double deletions...having some built-in pointers in the mix might be useful, but only if you are hyper-careful that they cannot possibly be used if their objects have been deleted, and you never, ever, use them to delete the object or otherwise exercise ownership of the object with them. My recommendation is to ... never mix them." — user870130, May 06 '14 at 18:43
@user870130: IMHO, that is bad advice. I think [the conventional wisdom from gurus like Herb Sutter](http://herbsutter.com/elements-of-modern-c-style/) is that smart pointers are great for "owning" pointers (or "optionally" owning in the case of `weak_ptr`) but raw pointers are still the best fit for non-owning pointers when you know the object will outlive the pointer. Smart pointers are no silver bullet, you still need a clear idea of ownership otherwise you will get problems, perhaps not dangling pointers or double deletions, but poor performance or memory leaks for example. — Chris Drew, May 07 '14 at 16:31

score 1 · Answer 1 · answered May 06 '14 at 17:01

class EventHandler {
private
    EventHandler(); //make the constructors protected, also in derived when possible

    template<class T, class...Us> //and make this function a friend
    friend std::shared_ptr<EventHandler> make_event(Us...us);
};
//this is the function you use to construct Event objects
template<class T, class...Us>
std::shared_ptr<T> make_event(Us...us)
{
    auto s = std::make_shared<T>(std::forward<Us>(us)...);
    EventDispatcher::getInstance().registerListener(s);
    return s;
}

This calls EventDispatcher's registerListener() member function which stores this in a vector.

void EventDispatcher::registerListener(std::weak_ptr<EventHandler> listener) 
{
   mListenerList.push_back(listener);
}

Where is mLisernerList looks like:

vector<std::weak_ptr<EventHandler>> mListenerList;

The EventDispatcher simply calls the sendEvent() on each element of the vector to notify it of the event.

Let me give an example to demonstrate my issue. Lets say my class Buttons inherits from EventHandler. I will create button objects on the heap and then place smart pointers to all my buttons in a single vector.

vector<std::shared_ptr<Buttons>> mButtons;   
mButtons.push_back(make_event<Buttons>());

score 1 · Answer 2 · answered May 06 '14 at 17:15

1

You can't naïvely use std::shared_ptr; there is some special support which might permit it, but it is overly complicated, and almost certainly not appropriate here; there's no way that an EventDispatcher would normally "own" an EventHandler.

The real question here is why you would want to use smart pointers at all here? The EventHandler registers in its constructor, and deregisters in its destructor. The pointers in EventDispatcher are purely for navigation. The same thing probably holds for mButtons, although there are designs where this might not be the case. (I tend to be a bit sceptical about vectors of unique_ptr. It depends on where the vector is situated, but from what I've seen, the additional complexity needed to access the actual pointers outweighs the complexity of handling the deletes manually.)

answered May 06 '14 at 17:15

James Kanze

150,581
18
184
329

I needed to create a few objects on the heap to keep them in scope. Then I wanted to keep them in a vector, so I could navigate them (you were right). Finally, I have a event handler system that I wanted to use. Each step seems so simple and logically. However, in practice this seems to make a mess. Can you suggest a better approach? – user870130 May 06 '14 at 17:59
2

The names (particularly `Button`) suggest a GUI. Typically, components of a GUI will belong to the component which contains them; additional components, like `EventHandler` (if not a base class of one of the GUI components) will belong to the class which they serve. Components which register somewhere will deregister in their destructor (and registries will use raw pointers). Thus, each object will belong to a single other object. You could use `unique_ptr` for this, but if these are in a vector, you might find it just as easy to do the necessary delete manually. – James Kanze May 06 '14 at 18:38

Can I use shared_ptrs in this example?

2 Answers2