Reputation: 412
I'm trying to implement the Observer pattern, but i don't want observers being responsible for my program safety by maintaining the reference list in ObservableSubject
.
Meaning that when the Observer
object
lifetime ends, I dont want to explicitly call ObservervableSubject::removeObserver(&object)
.
I have come up with an idea to use use pointer references in ObservableSubject
.
My question are: Is implementation described above and attempted below possible? And what is happening in my program, how do i prevent dereferencing trash?
Apriori excuse: This is an attempt at understanding C++, not something that should have actual use or be prefered over other implementations.
My solution attempt:
// Example program
#include <iostream>
#include <string>
#include <vector>
class ObserverInterface {
public:
virtual ~ObserverInterface() {};
virtual void handleMessage() = 0;
};
class ObservableSubject
{
std::vector<std::reference_wrapper<ObserverInterface*>> listeners;
public:
void addObserver(ObserverInterface* obs)
{
if (&obs)
{
// is this a reference to the copied ptr?
// still, why doesnt my guard in notify protect me
this->listeners.push_back(obs);
}
}
void removeObserver(ObserverInterface* obs)
{
// todo
}
void notify()
{
for (ObserverInterface* listener : this->listeners)
{
if (listener)
{
listener->handleMessage();
}
}
}
};
class ConcreteObserver : public ObserverInterface {
void handleMessage()
{
std::cout << "ConcreteObserver: I'm doing work..." << std::endl;
}
};
int main()
{
ObservableSubject o;
{
ConcreteObserver c;
o.addListener(&c);
}
o.notify();
std::cin.get();
}
Line in ObservableSubject::notify()
: Listener->handleMessage()
throws the following exception:
Exception thrown: read access violation.
listener->**** was 0xD8BF48B. occurred
Upvotes: 1
Views: 1045
Reputation: 206667
Your program has undefined behavior.
ObservableSubject o;
{
ConcreteObserver c;
o.addListener(&c); // Problem
}
c
gets destructed when the scope ends. You end up storing a stale pointer in the list of listeners of o
.
You can resolve the problem by defining c
in the same scope as o
or by using dynamically allocated memory.
ObservableSubject o;
ConcreteObserver c;
o.addListener(&c);
or
ObservableSubject o;
{
ConcreteObserver* c = new ConcreteObserver;
o.addListener(c);
}
When you use dynamically allocated memory, the additional scope is not useful. You might as well not use it.
ObservableSubject o;
ConcreteObserver* c = new ConcreteObserver;
o.addListener(c);
If you choose to use the second approach, make sure to deallocate the memory. You need to add
delete c;
before the end of the function.
You said:
Maybe i wasn't clear. Solving the lifetime/stale pointer problem was the intention of my solution. I know i have no problems if i have properly managed lifetime, or if i add
detachObserver
option on Observer destruction. I want to somehow be able to tell from theObservableSubject
if his list of Observers was corrupted, without the Observer explicitly telling that.
Since dereferencing an invalid pointer is cause for undefined behavior, it is essential that you track the lifetime of observers and make sure to update the list of observers when necessary. Without that, you are courting undefined behavior.
Upvotes: 5
Reputation: 19340
Note, I don't recommend the following approach, but I think it meets your requirements. You have a duplicated observer list. One is under control of the Observers, and the other, using weak pointers, is handled by the Observable object.
ObserverFactory
(which is their friend) to obtain a std::shared_ptr<Observer>
. The factory has a map from raw pointers to reference wrappers to the associated shared pointer.std::vector<std::weak_ptr<Observer>>
. On list traversal, you try to lock the weak_ptr; if it succeeds, handle the message; if it fails, that is, you get nullptr
, remove the weak pointer from the list.reset
on its shared pointer and remove from the map. This step is rather ugly, as it is just a fancy delete this
, normally a code smell.I believe you can also do this with std::shared_from_this
.
The plan is you move the maintenance away from the ObservableSubject back into the Observers.
Upvotes: 3
Reputation: 2647
// is this a reference to the copied ptr?
Yes, it is. It invokes undefined behaviour because the obs
pointer variable goes out of scope at the end of the function, resulting in a dangling reference.
The whole idea doesn’t gain you anything. Even if you make the ref-to-pointer approach work correctly, you are depending on one thing: That that exact pointer variable is set to nullptr
once the object dies. Essentially that’s the same problem as ensuring that no dangling pointers are held in listeners
.
For a heap object: How do you make sure that nobody deletes the object through a different pointer? Or forgets to null the registered pointer? It’s even worse for stack objects like in your example. The object goes out of scope and dies automatically. There is no opportunity to null anything unless you introduce an additional pointer variable that you’d have to manage manually.
You could consider two general alternatives to your approach:
Real-life implementations usually go in the general direction of leveraging C++ destructors for deregistration. Have a look at Qt’s signal/slot mechanism, for example.
Upvotes: 2