Michael Hamilton
2015-10-05 19:19:10 UTC
Abstract: observer_ptr as currently proposed does not "observe" its owner
and therefore should not be named observer. The current proposal should
choose a new name to reflect the raw_ptr ownerless nature which doesn't use
existing design pattern nomenclature. There is a use case for wanting to
detect expiration of a unique_ptr, however, and this could be
observer_ptr's purpose.
____
I recently discovered a proposal for observer_ptr
(http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2014/n3840.pdf)
It is correctly proposed as the "world's dumbest smart pointer" and I
believe the name itself is bad. I have no issue with the desire for a
self-documenting raw pointer wrapper, but observer is *not* an appropriate
name for such a device.
What does the term observer mean to you in programming? To me it means
something which observes state changes and receives notifications of those
changes.
Wikipedia agrees: https://en.wikipedia.org/wiki/Observer_pattern
"The *observer pattern* is a software design pattern
<https://en.wikipedia.org/wiki/Design_pattern_(computer_science)> in which
an object
<https://en.wikipedia.org/wiki/Object_(computer_science)#Objects_in_object-oriented_programming>,
called the subject, maintains a list of its dependents, called observers,
and notifies them automatically of any state changes, usually by calling
one of their methods
<https://en.wikipedia.org/wiki/Method_(computer_science)>."
This is an established pattern with established terminology, and for better
or worse, the term observer implies notification.
So, what's my beef?
std::unique_ptr is great for simple single-point ownership, it replaces
auto_ptr, is non-copyable, but is move-able.
std::shared_ptr is undeniably useful, but has many caveats in terms of
difficult to debug cyclic ownership scenarios needing to be carefully
managed with weak_ptr instances. The primary issue with shared_ptr is that
the lifetime is complicated and atomic reference counting needs to exist.
std::weak_ptr as mentioned above works hand in hand with shared_ptr. There
is a very handy "expired" method.
Raw pointers are still quite useful, but need to be carefully used in
places where ownership is guaranteed, and typically are the weak_ptr
equivalent to unique_ptr (ie: parent/child relationships with children
pointing to the parent.)
Why not std::raw_ptr, std::unowned_ptr, std::no_owner_ptr, or
std::ownerless_ptr, or std::orphan_ptr?
Why do I care? There is a use case that is not reflected in the above
pointer types which would be much better for using the term observer_ptr.
When one object controls lifespan, but other objects want to observe and
disconnect from the observed object when its lifespan expires! Right now we
are stuck with shared_ptr and weak_ptr and expired, but a cleaner interface
could certainly be created.
Imagine a std::unique_ptr being created, and then constructing something
like:
std::observer_ptr(const std::unique_ptr<T> &a_observed);
std::observer_ptr::expired() //returns true if the observed pointer is dead.
Optionally:
std::observer_ptr(const std::unique_ptr<T> &a_observed, const
std::function<void (T*)> &a_ondestroy); //call a_ondestroy when the
observed pointer is being destroyed.
What kind of use case is this good for? Signals/Slots automatically
deregistering themselves is one obvious example. I would imagine all the
instances of shared_ptr in the code sample below being unique_ptr and
observed_ptr.
Sometimes we want a single point of ownership, but *also* want to detect
when that ownership is broken. It seems like an obvious name for such a
mechanism would be observer_ptr, and it seems like a waste to use it on a
basic raw_ptr naming/self documenting case (especially when that naming is
confusing and misleading as I have already pointed out!)
Any thoughts?
#ifndef __MV_SIGNAL_H__
#define __MV_SIGNAL_H__
#include <memory>
#include <utility>
#include <functional>
#include <vector>
#include <set>
#include <string>
#include <map>
#include "Utility/scopeGuard.hpp"
namespace MV {
template <typename T>
class Reciever {
public:
typedef std::function<T> FunctionType;
typedef std::shared_ptr<Reciever<T>> SharedType;
typedef std::weak_ptr<Reciever<T>> WeakType;
static std::shared_ptr< Reciever<T> > make(std::function<T> a_callback){
return std::shared_ptr< Reciever<T> >(new Reciever<T>(a_callback, ++uniqueId));
}
template <class ...Arg>
void notify(Arg &&... a_parameters){
if(!isBlocked){
callback(std::forward<Arg>(a_parameters)...);
}
}
template <class ...Arg>
void operator()(Arg &&... a_parameters){
if(!isBlocked){
callback(std::forward<Arg>(a_parameters)...);
}
}
template <class ...Arg>
void notify(){
if(!isBlocked){
callback();
}
}
template <class ...Arg>
void operator()(){
if(!isBlocked){
callback();
}
}
void block(){
isBlocked = true;
}
void unblock(){
isBlocked = false;
}
bool blocked() const{
return isBlocked;
}
//For sorting and comparison (removal/avoiding duplicates)
bool operator<(const Reciever<T>& a_rhs){
return id < a_rhs.id;
}
bool operator>(const Reciever<T>& a_rhs){
return id > a_rhs.id;
}
bool operator==(const Reciever<T>& a_rhs){
return id == a_rhs.id;
}
bool operator!=(const Reciever<T>& a_rhs){
return id != a_rhs.id;
}
private:
Reciever(std::function<T> a_callback, long long a_id):
id(a_id),
callback(a_callback),
isBlocked(false){
}
bool isBlocked;
std::function< T > callback;
long long id;
static long long uniqueId;
};
template <typename T>
long long Reciever<T>::uniqueId = 0;
template <typename T>
class Signal {
public:
typedef std::function<T> FunctionType;
typedef Reciever<T> RecieverType;
typedef std::shared_ptr<Reciever<T>> SharedRecieverType;
typedef std::weak_ptr<Reciever<T>> WeakRecieverType;
//No protection against duplicates.
std::shared_ptr<Reciever<T>> connect(std::function<T> a_callback){
if(observerLimit == std::numeric_limits<size_t>::max() || cullDeadObservers() < observerLimit){
auto signal = Reciever<T>::make(a_callback);
observers.insert(signal);
return signal;
} else{
return nullptr;
}
}
//Duplicate Recievers will not be added. If std::function ever becomes comparable this can all be much safer.
bool connect(std::shared_ptr<Reciever<T>> a_value){
if(observerLimit == std::numeric_limits<size_t>::max() || cullDeadObservers() < observerLimit){
observers.insert(a_value);
return true;
}else{
return false;
}
}
void disconnect(std::shared_ptr<Reciever<T>> a_value){
if(a_value){
if(!inCall){
observers.erase(a_value);
} else{
disconnectQueue.push_back(a_value);
}
}
}
void block(std::function<T> a_blockedCallback = nullptr) {
isBlocked = true;
blockedCallback = a_blockedCallback;
calledWhileBlocked = false;
}
bool unblock() {
if (isBlocked) {
isBlocked = false;
return calledWhileBlocked;
}
return false;
}
bool blocked() const {
return isBlocked;
}
template <typename ...Arg>
void operator()(Arg &&... a_parameters){
if (!isBlocked) {
inCall = true;
SCOPE_EXIT{
inCall = false;
for (auto&& i : disconnectQueue) {
observers.erase(i);
}
disconnectQueue.clear();
};
for (auto i = observers.begin(); !observers.empty() && i != observers.end();) {
if (i->expired()) {
observers.erase(i++);
} else {
auto next = i;
++next;
i->lock()->notify(std::forward<Arg>(a_parameters)...);
i = next;
}
}
}
if (isBlocked) {
calledWhileBlocked = true;
if (blockedCallback) {
blockedCallback(std::forward<Arg>(a_parameters)...);
}
}
}
template <typename ...Arg>
void operator()(){
if (!isBlocked) {
inCall = true;
SCOPE_EXIT{
inCall = false;
for (auto&& i : disconnectQueue) {
observers.erase(i);
}
disconnectQueue.clear();
};
for (auto i = observers.begin(); i != observers.end();) {
if (i->expired()) {
observers.erase(i++);
} else {
auto next = i;
++next;
i->lock()->notify();
i = next;
}
}
}
if (isBlocked){
calledWhileBlocked = true;
if (blockedCallback) {
blockedCallback(std::forward<Arg>(a_parameters)...);
}
}
}
void setObserverLimit(size_t a_newLimit){
observerLimit = a_newLimit;
}
void clearObserverLimit(){
observerLimit = std::numeric_limits<size_t>::max();
}
int getObserverLimit(){
return observerLimit;
}
size_t cullDeadObservers(){
for(auto i = observers.begin();!observers.empty() && i != observers.end();) {
if(i->expired()) {
observers.erase(i++);
} else{
++i;
}
}
return observers.size();
}
private:
std::set< std::weak_ptr< Reciever<T> >, std::owner_less<std::weak_ptr<Reciever<T>>> > observers;
size_t observerLimit = std::numeric_limits<size_t>::max();
bool inCall = false;
bool isBlocked = false;
std::function<T> blockedCallback;
std::vector< std::shared_ptr<Reciever<T>> > disconnectQueue;
bool calledWhileBlocked = false;
};
//Can be used as a public SignalRegister member for connecting signals to a private Signal member.
//In this way you won't have to write forwarding connect/disconnect boilerplate for your classes.
template <typename T>
class SignalRegister {
public:
typedef std::function<T> FunctionType;
typedef Reciever<T> RecieverType;
typedef std::shared_ptr<Reciever<T>> SharedRecieverType;
typedef std::weak_ptr<Reciever<T>> WeakRecieverType;
SignalRegister(Signal<T> &a_slot) :
slot(a_slot){
}
SignalRegister(SignalRegister<T> &a_rhs) :
slot(a_rhs.slot) {
}
//no protection against duplicates
std::shared_ptr<Reciever<T>> connect(std::function<T> a_callback){
return slot.connect(a_callback);
}
//duplicate shared_ptr's will not be added
bool connect(std::shared_ptr<Reciever<T>> a_value){
return slot.connect(a_value);
}
void disconnect(std::shared_ptr<Reciever<T>> a_value){
slot.disconnect(a_value);
}
std::shared_ptr<Reciever<T>> connect(const std::string &a_id, std::function<T> a_callback){
return ownedConnections[a_id] = slot.connect(a_callback);
}
void disconnect(const std::string &a_id){
auto connectionToRemove = ownedConnections.find(a_id);
if (connectionToRemove != ownedConnections.end()) {
slot.disconnect(*connectionToRemove);
}
}
bool connected(const std::string &a_id) {
return ownedConnections.find(a_id) != ownedConnections.end();
}
private:
std::map<std::string, SharedRecieverType> ownedConnections;
Signal<T> &slot;
};
}
#endif
and therefore should not be named observer. The current proposal should
choose a new name to reflect the raw_ptr ownerless nature which doesn't use
existing design pattern nomenclature. There is a use case for wanting to
detect expiration of a unique_ptr, however, and this could be
observer_ptr's purpose.
____
I recently discovered a proposal for observer_ptr
(http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2014/n3840.pdf)
It is correctly proposed as the "world's dumbest smart pointer" and I
believe the name itself is bad. I have no issue with the desire for a
self-documenting raw pointer wrapper, but observer is *not* an appropriate
name for such a device.
What does the term observer mean to you in programming? To me it means
something which observes state changes and receives notifications of those
changes.
Wikipedia agrees: https://en.wikipedia.org/wiki/Observer_pattern
"The *observer pattern* is a software design pattern
<https://en.wikipedia.org/wiki/Design_pattern_(computer_science)> in which
an object
<https://en.wikipedia.org/wiki/Object_(computer_science)#Objects_in_object-oriented_programming>,
called the subject, maintains a list of its dependents, called observers,
and notifies them automatically of any state changes, usually by calling
one of their methods
<https://en.wikipedia.org/wiki/Method_(computer_science)>."
This is an established pattern with established terminology, and for better
or worse, the term observer implies notification.
So, what's my beef?
std::unique_ptr is great for simple single-point ownership, it replaces
auto_ptr, is non-copyable, but is move-able.
std::shared_ptr is undeniably useful, but has many caveats in terms of
difficult to debug cyclic ownership scenarios needing to be carefully
managed with weak_ptr instances. The primary issue with shared_ptr is that
the lifetime is complicated and atomic reference counting needs to exist.
std::weak_ptr as mentioned above works hand in hand with shared_ptr. There
is a very handy "expired" method.
Raw pointers are still quite useful, but need to be carefully used in
places where ownership is guaranteed, and typically are the weak_ptr
equivalent to unique_ptr (ie: parent/child relationships with children
pointing to the parent.)
Why not std::raw_ptr, std::unowned_ptr, std::no_owner_ptr, or
std::ownerless_ptr, or std::orphan_ptr?
Why do I care? There is a use case that is not reflected in the above
pointer types which would be much better for using the term observer_ptr.
When one object controls lifespan, but other objects want to observe and
disconnect from the observed object when its lifespan expires! Right now we
are stuck with shared_ptr and weak_ptr and expired, but a cleaner interface
could certainly be created.
Imagine a std::unique_ptr being created, and then constructing something
like:
std::observer_ptr(const std::unique_ptr<T> &a_observed);
std::observer_ptr::expired() //returns true if the observed pointer is dead.
Optionally:
std::observer_ptr(const std::unique_ptr<T> &a_observed, const
std::function<void (T*)> &a_ondestroy); //call a_ondestroy when the
observed pointer is being destroyed.
What kind of use case is this good for? Signals/Slots automatically
deregistering themselves is one obvious example. I would imagine all the
instances of shared_ptr in the code sample below being unique_ptr and
observed_ptr.
Sometimes we want a single point of ownership, but *also* want to detect
when that ownership is broken. It seems like an obvious name for such a
mechanism would be observer_ptr, and it seems like a waste to use it on a
basic raw_ptr naming/self documenting case (especially when that naming is
confusing and misleading as I have already pointed out!)
Any thoughts?
#ifndef __MV_SIGNAL_H__
#define __MV_SIGNAL_H__
#include <memory>
#include <utility>
#include <functional>
#include <vector>
#include <set>
#include <string>
#include <map>
#include "Utility/scopeGuard.hpp"
namespace MV {
template <typename T>
class Reciever {
public:
typedef std::function<T> FunctionType;
typedef std::shared_ptr<Reciever<T>> SharedType;
typedef std::weak_ptr<Reciever<T>> WeakType;
static std::shared_ptr< Reciever<T> > make(std::function<T> a_callback){
return std::shared_ptr< Reciever<T> >(new Reciever<T>(a_callback, ++uniqueId));
}
template <class ...Arg>
void notify(Arg &&... a_parameters){
if(!isBlocked){
callback(std::forward<Arg>(a_parameters)...);
}
}
template <class ...Arg>
void operator()(Arg &&... a_parameters){
if(!isBlocked){
callback(std::forward<Arg>(a_parameters)...);
}
}
template <class ...Arg>
void notify(){
if(!isBlocked){
callback();
}
}
template <class ...Arg>
void operator()(){
if(!isBlocked){
callback();
}
}
void block(){
isBlocked = true;
}
void unblock(){
isBlocked = false;
}
bool blocked() const{
return isBlocked;
}
//For sorting and comparison (removal/avoiding duplicates)
bool operator<(const Reciever<T>& a_rhs){
return id < a_rhs.id;
}
bool operator>(const Reciever<T>& a_rhs){
return id > a_rhs.id;
}
bool operator==(const Reciever<T>& a_rhs){
return id == a_rhs.id;
}
bool operator!=(const Reciever<T>& a_rhs){
return id != a_rhs.id;
}
private:
Reciever(std::function<T> a_callback, long long a_id):
id(a_id),
callback(a_callback),
isBlocked(false){
}
bool isBlocked;
std::function< T > callback;
long long id;
static long long uniqueId;
};
template <typename T>
long long Reciever<T>::uniqueId = 0;
template <typename T>
class Signal {
public:
typedef std::function<T> FunctionType;
typedef Reciever<T> RecieverType;
typedef std::shared_ptr<Reciever<T>> SharedRecieverType;
typedef std::weak_ptr<Reciever<T>> WeakRecieverType;
//No protection against duplicates.
std::shared_ptr<Reciever<T>> connect(std::function<T> a_callback){
if(observerLimit == std::numeric_limits<size_t>::max() || cullDeadObservers() < observerLimit){
auto signal = Reciever<T>::make(a_callback);
observers.insert(signal);
return signal;
} else{
return nullptr;
}
}
//Duplicate Recievers will not be added. If std::function ever becomes comparable this can all be much safer.
bool connect(std::shared_ptr<Reciever<T>> a_value){
if(observerLimit == std::numeric_limits<size_t>::max() || cullDeadObservers() < observerLimit){
observers.insert(a_value);
return true;
}else{
return false;
}
}
void disconnect(std::shared_ptr<Reciever<T>> a_value){
if(a_value){
if(!inCall){
observers.erase(a_value);
} else{
disconnectQueue.push_back(a_value);
}
}
}
void block(std::function<T> a_blockedCallback = nullptr) {
isBlocked = true;
blockedCallback = a_blockedCallback;
calledWhileBlocked = false;
}
bool unblock() {
if (isBlocked) {
isBlocked = false;
return calledWhileBlocked;
}
return false;
}
bool blocked() const {
return isBlocked;
}
template <typename ...Arg>
void operator()(Arg &&... a_parameters){
if (!isBlocked) {
inCall = true;
SCOPE_EXIT{
inCall = false;
for (auto&& i : disconnectQueue) {
observers.erase(i);
}
disconnectQueue.clear();
};
for (auto i = observers.begin(); !observers.empty() && i != observers.end();) {
if (i->expired()) {
observers.erase(i++);
} else {
auto next = i;
++next;
i->lock()->notify(std::forward<Arg>(a_parameters)...);
i = next;
}
}
}
if (isBlocked) {
calledWhileBlocked = true;
if (blockedCallback) {
blockedCallback(std::forward<Arg>(a_parameters)...);
}
}
}
template <typename ...Arg>
void operator()(){
if (!isBlocked) {
inCall = true;
SCOPE_EXIT{
inCall = false;
for (auto&& i : disconnectQueue) {
observers.erase(i);
}
disconnectQueue.clear();
};
for (auto i = observers.begin(); i != observers.end();) {
if (i->expired()) {
observers.erase(i++);
} else {
auto next = i;
++next;
i->lock()->notify();
i = next;
}
}
}
if (isBlocked){
calledWhileBlocked = true;
if (blockedCallback) {
blockedCallback(std::forward<Arg>(a_parameters)...);
}
}
}
void setObserverLimit(size_t a_newLimit){
observerLimit = a_newLimit;
}
void clearObserverLimit(){
observerLimit = std::numeric_limits<size_t>::max();
}
int getObserverLimit(){
return observerLimit;
}
size_t cullDeadObservers(){
for(auto i = observers.begin();!observers.empty() && i != observers.end();) {
if(i->expired()) {
observers.erase(i++);
} else{
++i;
}
}
return observers.size();
}
private:
std::set< std::weak_ptr< Reciever<T> >, std::owner_less<std::weak_ptr<Reciever<T>>> > observers;
size_t observerLimit = std::numeric_limits<size_t>::max();
bool inCall = false;
bool isBlocked = false;
std::function<T> blockedCallback;
std::vector< std::shared_ptr<Reciever<T>> > disconnectQueue;
bool calledWhileBlocked = false;
};
//Can be used as a public SignalRegister member for connecting signals to a private Signal member.
//In this way you won't have to write forwarding connect/disconnect boilerplate for your classes.
template <typename T>
class SignalRegister {
public:
typedef std::function<T> FunctionType;
typedef Reciever<T> RecieverType;
typedef std::shared_ptr<Reciever<T>> SharedRecieverType;
typedef std::weak_ptr<Reciever<T>> WeakRecieverType;
SignalRegister(Signal<T> &a_slot) :
slot(a_slot){
}
SignalRegister(SignalRegister<T> &a_rhs) :
slot(a_rhs.slot) {
}
//no protection against duplicates
std::shared_ptr<Reciever<T>> connect(std::function<T> a_callback){
return slot.connect(a_callback);
}
//duplicate shared_ptr's will not be added
bool connect(std::shared_ptr<Reciever<T>> a_value){
return slot.connect(a_value);
}
void disconnect(std::shared_ptr<Reciever<T>> a_value){
slot.disconnect(a_value);
}
std::shared_ptr<Reciever<T>> connect(const std::string &a_id, std::function<T> a_callback){
return ownedConnections[a_id] = slot.connect(a_callback);
}
void disconnect(const std::string &a_id){
auto connectionToRemove = ownedConnections.find(a_id);
if (connectionToRemove != ownedConnections.end()) {
slot.disconnect(*connectionToRemove);
}
}
bool connected(const std::string &a_id) {
return ownedConnections.find(a_id) != ownedConnections.end();
}
private:
std::map<std::string, SharedRecieverType> ownedConnections;
Signal<T> &slot;
};
}
#endif
--
---
You received this message because you are subscribed to the Google Groups "ISO C++ Standard - Discussion" group.
To unsubscribe from this group and stop receiving emails from it, send an email to std-discussion+***@isocpp.org.
To post to this group, send email to std-***@isocpp.org.
Visit this group at http://groups.google.com/a/isocpp.org/group/std-discussion/.
---
You received this message because you are subscribed to the Google Groups "ISO C++ Standard - Discussion" group.
To unsubscribe from this group and stop receiving emails from it, send an email to std-discussion+***@isocpp.org.
To post to this group, send email to std-***@isocpp.org.
Visit this group at http://groups.google.com/a/isocpp.org/group/std-discussion/.