I'm bored of the argument now. We both clearly have different use cases for shared_ptr and mine isn't compatible with your use cases for bind. I use shared_ptr for objects which are transient, and may be deleted at any time. I use weak_ptr when storing a reference to any object A in a shared_ptr, within any object B which does not own the object. The idea being that, when the owning object disposes of A all objects B will have an invalid weak_ptr and an act accordingly when they later come to access the object. shared_ptr and weak_ptr are the way I guarantee de-allocation correctness, and avoid memory leaks.

In the case of a messaging system, objects of type A will give objects of type B a weak_ptr to themselves. When object B comes to pass a message down the pipeline, it can lock the weak_ptr and send the message, unlocking the pointer when it is done.

When object A dies, its destructor will be called, because the shared_ptr is only held in objects which I want to keep A alive. object A can remove itself from object B in its destructor. Everybody is happy. As an insurance policy against inevitable mistakes, when object B finds a dead weak_ptr, it knows it no longer has a valid object and can act accordingly. In the case of a message system, this will involve either a swap-delete, or re-use of that slot later.

Now, I don't know if this is the "wrong" use of shared_ptr any more than I know if a hacksaw is the "wrong" tool to cut a loaf of bread with, but in an environment lacking a perfect tool I have to bend an existing tool to my will. Anybody who is upset by this can either buy me a bread knife, or suck it up


Here are the use cases I gleaned from reading the thread and the OP's responses to suggestions:
Object subscribes to an individual message
Object subscribes to a whole channel of messages
Messages are only sent to objects which care about messages in that category
The smallest possible overhead

I added the following use cases as a matter of principle:
Simplicity of understanding - OP can go and implement it in a few minutes and not spend time wranging with compiler versions or boost
Compatibiltiy with shared_ptr

To summarise, your solution disturbs me in a few ways:

Use of bind and function which increases the amount of code and conceptual overhead
Increased complexity at the recieving end, since one must bind many function pointers, and subscribe to every unique event.
No support for event channels
Objects are bound many times and the mechanism for unbinding an object is more complicated. It doesn't seem to have really been considered.
Incompatibility with shared_ptr
High sensitivity to mistakes such as not unbinding all bound member functions in the destructor

My method suffers from a single overriding problem, the requirement of void * as a payload.

The issue of a static list of message types is shared by both systems, but one could implement either as a system of type ID's instead, perhaps with a system of primary and secondary type ID's to simplify message routing.

OK, I think now we are getting somewhere and we have the 'root of the problem' as it where.

We both clearly have different use cases for shared_ptr and mine isn't compatible with your use cases for bind.

The problem, as it turns out, isn't the use cases; I agree with your general use case, specifically when it comes to handing out weak_ptrs to observing instances of a class.

The 'problem' is this;

As an insurance policy against inevitable mistakes, when object B finds a dead weak_ptr, it knows it no longer has a valid object and can act accordingly. In the case of a message system, this will involve either a swap-delete, or re-use of that slot later.
[/quote]

The problem is not that 'shared_ptr is incompatible with bind/function' it is that 'because of my design choice I can't use shared_ptr'.

And your solution is sub-optimal, but then most cases where you 'protect' the user are because it creates more work AND lets them do The Wrong Thing™. Even if you stick with your solution you shouldn't just be going 'oh well...' and cleaning up; you should assert and make a noise about the fact that 'hey, you have screwed up, fix the problem'.

So, your usage is fine, the problem is your design choice isn't the best for this perticular problem.

Here are the use cases I gleaned from reading the thread and the OP's responses to suggestions:
Object subscribes to an individual message
Object subscribes to a whole channel of messages
Messages are only sent to objects which care about messages in that category
The smallest possible overhead
[/quote]

ALL of which are serviced by the system I proposed and served BETTER than your solution.

Leaving aside the shared_ptr issues, which I'll come back to in a moment, it stacks up as following;

key;
{ } = optional stages

- Object subscribes to an individual message
mine: object registers intrest with message system and is called when a message arrives at the correct hub. Object only invoked once per message, if multiple objects are intrested in the message a loop takes care of it.
[message system]-->[X instances]

yours: object requires some method of figuring out 'what' message it has been given OR sits behind a 'filter' which only forwards one message. In the former case you get called X amount of times when there are X messages in the queue, requires 'if' or 'switch' to correctly route/deal with intrested message. For latter case filter is called X amount of times when there are X messages in the queue, requires 'if' or 'switch' to correctly reject unintrested messages before forwarding to intrested party.
[message system]-->[filter]-->{[relay]}-->[{X} instances]
or
[message system]-->{[relay]-->}[{X} instances] where each instance filters messages.

- Object subscribes to a whole channel of messages
mine: object register intrest with message system for X messages and is called when message arrives at the correct hub. Object invoked once per message, if multiple objects are intrested in messag a loop takes care of it. 'Channel' registration is nothing more than either a loop or an extra function for registering covering one or more of these situations
- a range of messages and one call back function - receiving instance required to figure out message
- an array of message IDs and one call back function - receiving instance required to figure out message
- an array of message ID to callback function pairs - arguably more optimal as it removes requirement on reciving class.
List of intrested messages remain in the class which is registering them. Also can be made dynamic if required at runtime.
[message system]-->[X instances]

yours: filter class is required, either requiring static 'switch' statement/if check or some form of dynamic array if you want to reuse the class for multiple ranges of 'channels'. When a message is sent the idea is checked via either a switch/if statement OR via some form of dynamic look up. The message is then forward down to the next class in the chain; if multiple recievers are required it must pass into a relay class. Recieving instances needs to figure out what message in order to deal with them.
[message system]-->[filter]-->{[relay]}-->[{X} instances]

- Messages are only sent to objects which care about messages in that category
mine: object registers intrest with message system and is called when a message arrives at the correct hub. Object only invoked once per message, if multiple objects are intrested in the message a loop takes care of it.
[message system]-->[X instances]

yours: filter class is required, either requiring static 'switch' statement/if check or some form of dynamic array if you want to reuse the class for multiple ranges of 'channels'. When a message is sent the idea is checked via either a switch/if statement OR via some form of dynamic look up. The message is then forward down to the next class in the chain; if multiple recievers are required it must pass into a relay class.
[message system]-->[filter]-->{[relay]}-->[{X} instances]

- The smallest possible overhead
Runtime;
mine: map lookup, most likely a red-black tree, O(log n) for message types + O(n) for parties intrested in message

yours: depends on length of chain. O(n) for intrested parties per relay class instance in chain + O(1) or O(log n) or O(n) per filter class, depending on method used to filter and compiler.

Code;
mine: no extra classes required beyond base message dispatcher
yours: at least 2 extra classes beyond base message dispatcher, increasing depending on filter implimentation, O(1) requires extra filter implimentation per grouping, slower filters require no extra classes.

So, from this comparison we can see the following;

- Design wise and runtime wise;

Mine is simpler. It requires no extra classes, dispatch is handled in a single place and covers all requirements given without added complexity at dispatch point. 'Channels' are handled by a simple interface tweak and maintains information in intrested party.

Yours, on the other hand, requires at least two extra classes to filter and broadcast with associated code matainance overhead and chance for errors and depending on filtering methods this increases both in terms of code maintainence and potential problem points. Dispatch while central can be stopped at any given point and without extra filtering can invoke instances when there is no requirement to.

- Runtime cost wise

This is a bit of a crap shoot however this is more on your side than mine; mine is easy to predict due to the nature of it.
Yours however becomes harder as it depends on chain lenght, filtering methods and also includes function call overhead for every step in the chain. Your final 'intrested party' is still O(n) but only O(n) for everything below the last filter, which could include instance which get invoked when not required if the filtering isn't tight enough above them.

In short, at this point mine remains a better solution, it is more predictable and has less code and thus less maintainance associated with it.

Simplicity of understanding - OP can go and implement it in a few minutes and not spend time wranging with compiler versions or boost
Compatibiltiy with shared_ptr
[/quote]

On the surface it might seem simple but as shown above your solution has hidden complexity depending on usage case. Even with channel the only extra complexity comes at a minor interface change. On the flip side upgrading to VS2010 is easy and Boost is such a useful tool (not in all cases before anyone who works on consoles hits me!) that NOT knowing about it, or at least the more useful 'header only' functionality is a bad thing.

Compatibility with shared_ptr depends very much on YOUR implimentation detail and thus isn't important frankly, but as I said I'll come back to that

1. Use of bind and function which increases the amount of code and conceptual overhead
2. Increased complexity at the recieving end, since one must bind many function pointers, and subscribe to every unique event.
3. No support for event channels
4. Objects are bound many times and the mechanism for unbinding an object is more complicated. It doesn't seem to have really been considered.
5. Incompatibility with shared_ptr
6. High sensitivity to mistakes such as not unbinding all bound member functions in the destructor
[/quote]

1. Disagree; as shown above it reduces code requirements and given that bind and function are an active part of the standard spending a short amount of time learning them is a minor problem. Frankly if you are a programmer who DOESN'T want to stay up to date then you've got bigger problems.

2 & 3; Depends on interface to message system; multiple bindings are not that hard to do;
[source]
void MessageSystem::Register(int messageStart, int messageEnd, functionObjectType const &func);
void MessageSystem::Register(int const * messages, int count, functionObjectType const &func);
void MessageSystem::Register(std::vector<int> const &message, functionObjectType const &func);
struct
{
int message;
functionObjectType func;
} Messagebinder;

void MessageSystem::Register(MessageBinder const * message, int count);
void MessageSystem::Register(std::vector<MessageBinder> const &messages);

void Func(MessageSystem &msgSys)
{
MessageBinder mb = { {msg1, bar}, {msg2, bar}, {msg2, foo} };
msgSys.Register(mb, 3);
}
[/source]
Pick an interface, any interface.. the point is the details about what a class is intrested in remain with that class, not hidden away in some filter object.

4. I conceed that this is an issue, however it isn't a major issue depending on how you want to handle it. I will in fact come back to this point as it dove tails nicely with my point about your design above Also, while you complain about my lack of removal code I've not seen anything which really explains how you setup and link all these "wonderful" filters and relays together and then make sure objects end up at the correct points in these chains.

5. Already covered; this is down to your design not a problem with the system.

6. This is a GOOD thing. You should not be covering for mistakes, you should be blowing up and making people fix them. However with the correct 'clean up' scheme this becomes a trival thing.

My method suffers from a single overriding problem, the requirement of void * as a payload.
[/quote]

*chuckles* yours suffers from a lot more than that as covered, however it should be noted I never complained about the void*; mostly because I've not come up with a better solution myself yet and I'm not sure one can really exist outside of using something like boost::varient but then you just shift the problem.

Which brings me, finally, to why shared_ptr ISN'T a problem but your design is.

So, right away, let me get something off my chest; in an earlier reply you complained about the overhead of a std::map lookup but you are completely ok with puting N weak_ptr validations in your main dispatch loop? In the grand scheme of things that seem a little strange to say the least because thats going to be much heavier than a lookup very quickly.

Also, you are once again focusing on the wrong place performance wise. You want your dispatch point to be as fast as possible, which is why a simple 'look up and loop' solution is going to trump any solution where you have to validate object pointers every step of the way.

Instead we shift our attention back to what are, on average, going to be very low frequency operations; adding and removing.

As shown above adding, both in multiple and single terms is pretty trivial; to get one object to listen to one message requires it register itself for that one message and multiple messages can be handled just as simply.

Removal, well that becomes a little more tricky however given you haven't explained how your stuff hooks together I think we are above even there at this point, however I do have a solution to the RAII cleanup question.

As it stands right now Register doesn't return anything, however we can change this and at the same time take care of the clean up problem. It does require that we invent an extra class however;

[source]
// This is just an example, implimentation depends on how the MessageSystem is done
// This solution works due to std::list not invalidating iterators when objects are added/removed from a list
struct
{
int message;
functionObjectType func;
} Messagebinder;

class MessageSystem
{
typedef std::function<void (payloadType*)> MessageType;
typedef std::list<MessageType> MessageList;
typedef std::pair<int, MessageList::Iterator> MessageHandlerPair;
typedef std::vector<MessageHandlerPair> MessageHandlerStore;
typedef std::map<MessageType, MessageList> MessageMap

MessageList messageMap;

HandlerStore Register(std::vector<MessageBinder> const &messages)
{
MessageHandlerStore storedMessageHandlers;
std:foreach(message.begin(), messages.end(), [&messageMap, &storedMessageHandlers](MessageBinder &mb)
{
MessageList &list = messageMap[mb.message];
MessageList::Iterator itr = list.insert(list.begin(), mb.func);
storedMessageHandlers.pushBack(std::make_pair(mb.message, itr));
});
return HandlerStore(this, storedMessageHandlers);
}

void UnRegister(MessageHandlerStore const & handlerStore)
{
std::foreach(handlerStore.begin(), handlerStore.end(), [&messageMap](MessageHandlerPair &hp)
{
MessageMap::Iterator itr = messageMap.find(hp.first);
if(itr != messageMap.end())
{
itr->erase(hp.second);
}
});
}

void Dispatch(MessageType msgId, payloadType *msg)
{
MessageMap::Iterator Itr = messageMap.find(msgId);
if(Itr != messageMap.end() && !Itr->empty())
{
std::for_each(Itr->begin(), Itr->end(), [&msg](MessageType &func) { func(msg); });
}
}
}

class HandlerStore
{
MessageSystem::MessageHandlerStore handlers;
MessageSystem *msgSys;

HandlerStore() : msgSys(0) {}
HandlerStore(MessageSystem const *msgSys, MessageSystem::MessageHandlerStore &handlers) : handlers(handlers), msgSys(msgSys) {}
~HandlerStore()
{
Release();
}

void Release()
{
if(msgSys && !handlers.empty())
{
msgSystem.UnRegister(handlers);
handlers.clear();
}
}
}

class Foo
{
HandlerStore store;

void Bar(payloadType *) {};
void Wibble(payloadType *) {};

void Init(MessageSystem * msgSys)
{
std::vector<Messagebinder> mb;
mb.push_back(std::make_pair(msg1, std::bind(Foo::Bar, this, _1);
mb.push_back(std::make_pair(msg2, std::bind(Foo::Bar, this, _1);
mb.push_back(std::make_pair(msg3, std::bind(Foo::Wibble, this, _1);

store = msgSys->Register(mb);
}
}
[/source]

Pretty standard C++ fare, RAII style clean up when object goes out of scope, no change of mistakes. If the user fails to take ownership of the HandlerStore object then the messages are cleaned up right away so you can't "dangle" them. Requires that the message system stay in scope, however if that happens you have a design fault which needs to be corrected.

This could also be adjusted so that it uses an std::vector or std::deque instead of a list, however at this point you need to change the stored message type so that it also holds a handle and then store that handle in the 'HandleStore' type and use that to select which elements to remove. This is slower but it does mean your function objects stay contiguous during dispatch so YMMV.

Supports;
- range of message subscription (covers 'channels')
- filtering via lack of subscription
- low overhead dispatch with no coupling
- RAII style clean up and deregistering as well as manual deregistering
- Does not require setup of filter/relay chains by some external entity; instances are self managing

Required Knowledge;
- Standard C++ containers
- Standard C++ algorithms
- boost::function or std::function (or some other custom solution, but I'd stick with the standard solutions)
- Optionally C++ Lambda functionality, although those loops could be replace with 'for' or 'while' loops with no problems
[/quote]

What all this shows is that your solution can definately be applied to a variety of situations. I would like to reiterate that, in an X vs Y debate I often say "both lol". Now, I have a scalpel for cutting my bread, as well as my hacksaw.

Regarding the scenario of testing the weak_ptr, I have considered various failure behaviours, including logging, exceptions, and silence. I lean towards a combination of logging and exceptions, so, debug builds throw an exception while a release build just logs the error. The mistake should be detected at all costs and a programmer notified during a debug run, but when a user is playing, they don't want the game to crash every few minutes due to recoverable errors. To ensure your users report the problem, an error dialog between play can be an option.