On Tue, Mar 25, 2008 at 4:41 PM, Steven Watanabe wrote:

AMDG

Robert Dailey wrote:

...

Hi,

Currently I'm using Boost.Signals to transfer Packet objects to subscribers. Right now my Boost.Signal looks like below:

boost::signal

Right now Packet is the base class for several other packet types, each having the specific data a subscriber needs access to. Of course, given this design, the only way to obtain the data is to perform a downcast via static_cast or dynamic_cast:

WalkPacket const& wp = static_cast( p ); // 'p' here is a generic Packet object.

This is obviously bad design since it isn't type safe at all. It's also repetitive and tedious since each subscriber must duplicate the logic to perform the downcast. I'm wondering if there's anything in Boost you guys can suggest that I use to make this a little better. I would prefer to have a common Signal object that can take differently typed Slot objects. For example, I want to pass in a slot that looks like: "void (WalkPacket& p)" and have the signal utilize implicit conversions or something. Templates aren't even being used here because I can't find a way to fit them in, since everything is being done at runtime and not compile time.

In this case you want the packets that are not WalkPackets to be ignored? You could wrap the slots in a class like this: (untested)

template struct downcaster { typedef boost::remove_reference<T> arg_type; downcaster(F f) : f(f) {} template<class U> void operator()(const U& u) const { if(arg_type* arg = dynamic_cast(&u)) { f(*arg); } } F f; }; template downcaster make_downcaster(F f) { return(downcaster(f)); }

In Christ, Steven Watanabe

Sorry, I guess I didn't explain myself very well. Your class is cute but I don't think it is going to work, or perhaps I am misunderstanding its purpose. It would be great if you could show me how the boost::signal would be defined using your code above. Basically, I have several types of packets. For example: WalkPacket, ChatPacket, QuestPacket, etc. When I'm receiving data from the network, I first accumulate a buffer of bytes and deserialize that into one of these packet structures. The data I'm receiving from the network will have an ID in there which maps to one of these types. For example, ID 1 would be WalkPacket, ID 2 is ChatPacket, and so forth. This is what my packet factory does, it takes an ID and returns a Packet* (which represents the actual concrete type that maps to the ID). When these packets are received and created, they are immediately sent to subscribers interested in that packet and then the packet is destroyed. Before I said I wanted to only use 1 boost.signal object, however I was incorrect. I need 1 boost.signal for each packet type. So I guess then what I need is a type of map that can take an integral as the key value and is capable of containing heterogeneously typed values. Boost.Fusion won't work here, since it does the exact opposite: it maps types to values (of course you could probably do some template tricks to get this to work). And the MPL library won't work either since it is strictly compile time, and we never know what ID's we're going to use until runtime. I need to be able to construct a map like this: enum PacketIDs { PID_WALKPACKET = 0, PID_CHATPACKET, PID_QUESTPACKET }; // This is not a map of types! map< pair >, pair >, pair >

m_signalMap;

// And then to send the packet, at runtime we simply do: WalkPacket myWalkPacket; // Note that the value used to index into the map won't be so static in the real use case. m_signalMap[PID_WALKPACKET]( myWalkPacket ); So I guess the real question is: Does boost have such a map? Note that Boost.Fusion map would probably work great for this, however I'm stuck using boost 1.34.1, which doesn't have boost::fusion::map. If you're confused about my first post, that's because I was too. My first design choice was to use polymorphism, however that obviously won't work since we only want to dispatch packets to slots that are interested in that specific packet. Having 1 global signal for all packet subscribers is too wasteful. Doing it this way also enables us to be a little more type safe on the subscriber's end at least.

Steven Watanabe wrote:

You might also look into replacing Packet with boost::variant.

I would second this. If you use: typedef boost::variant Packet; You might also consider using pointers in your Packet type, or even shared_ptr<WalkPacket>, etc, but that is a detail which depends on how the rest of your application is built. I use pointer-like types because I am very aware of making copies of my potentially-large and numerous packets. In this situation, all of your messaging infrastructure that deals with packets passes around instances of the variant type. As you have described, your clients have a mechanism where they can register using wrapper functions around the signals library to receive notification on the arrival of specific packet types - one signal for each packet type. When it comes time to distribute your packet, you can make use of the boost::variant visitor feature, which allows you to select operations based on the actual type of the data in your variant instance. In your case it might look something like this: // Your packet type typedef boost::variant Packet; // Your signals... boost::signal distributeWalkerPacket; boost::signal distributeChatPacket; // Helper class for apply_visitor class doDistributePacket : public static_visitor<> { public: void operator() ( WalkPacket p ) const { distributeWalkPacket( p ); } void operator() ( ChatPacket p ) const { distributeChatPacket( p ); } // and so on ... }; // Free function to drive this above. void DistributePacket( Packet p ) { apply_visitor( doDistributePacket(), p ); } I like boost::variant because it tends to bring all the type-dependent code together and makes it easy to encapsulate type based decisions. Kevin

AMDG Robert Dailey wrote:

I love the logic presented here. boost::variant has always been confusing to me, but I took the time just now to read over the documentation for it a few times and I get it now. However, the issue still remains in how to organize all of my signals. In your example you have all of the signals manually typed out into separate variables, whereas I would rather have them in some sort of container, such as a map.

How about this: typedef boost::mpl::vector packet_types; typedef boost::mpl::transform >::type signals; typedef boost::mpl::inherit_linearly, mpl::inherit<_, _> >::type SignalVisitor; In Christ, Steven Watanabe

AMDG Robert Dailey wrote:

Would you mind going over what exactly this is doing? I'm having a hard time wrapping my head around it. The documentation isn't helping much either, as I've never used these before and the descriptions are very confusing.

I also didn't see a version of mpl::transform that only took 1 template argument. Is this correct?

You're right. I was in too much of a hurry before. Here's something that actually compiles. #include #include #include #include #include #include namespace mpl = boost::mpl; // bring _ into scope. using namespace mpl::placeholders; struct WalkPacket {}; struct ChatPacket {}; struct QuestPacket {}; // a metafunction to create a signal type that // takes T as is argument by const reference. template<class T> struct make_signal { typedef boost::signal type; }; // a type list of all the possible packet types. typedef mpl::vector packet_types; // take the list of packet types and transform it // to get the signal types. The result will be // equivalent to // mpl::vector, signal, signal > // // make_signal<_> is an mpl lambda expression. _ // is a placeholder which in this context means // to substitute the first argument to the // metafunction. In other words mpl::apply, T> // will replace _ with T and then return the nested ::type // typename make_signal<T>::type. typedef mpl::transform >::type signals; // A type that inherits from all of the signal types. // // The mpl::inherit<_, _> is similar to make_signal<_> above, // except that the first _ refers to the first argument and // and the second _ refers to the second argument. // mpl::inherit_linearly starts with an initial base class // which defaults to mpl::empty_base, takes the // first type in signals, and applies mpl::inherit to // the two bases. inherit. // Then, it continues with the second element of signals // to get inherit, ChatPacket> // And so on. // inherit just inherits from both it's arguments. // // This could really be replaced by a fusion::set. typedef mpl::inherit_linearly >::type signal_holder; struct signal_visitor : boost::static_visitor<> { explicit signal_visitor(signal_holder& holder) : holder_(holder) {} signal_holder& holder_; template<class T> void operator()(T const& t) const { // up-cast to correct type of signal. return(static_cast(holder_)(t)); } }; typedef boost::make_variant_over::type packet; int main() { packet p = WalkPacket(); signal_holder signals; boost::apply_visitor(signal_visitor(signals), p); } In Christ, Steven Watanabe

AMDG Robert Dailey wrote:

The term 'inherit' here does not refer to class inheritance obviously. It's an unusual usage of the term to me. When you do inherit what exactly is that doing?

template struct inherit : T1, T2 {};

Is it joining them in a list container or something? What is the resulting type of mpl::inherit_linearly::type? Is the result of mpl::inherit_linearly::type implicitly constructible from an mpl::transform::type?

The result of the inherit_linearly call inherits using multiple inheritence from all the types in the sequence.

I'm going to read over the docs a few more times in the meantime to see if I can grasp this concept. Also, I'm not seeing how you're filling in the 2 placeholders in the signal_holder typedef.

mpl::inherit_linearly treats the second argument as a binary lambda expression. It is very similar to the runtime std::accumulate. Think of inherit<_, _> as the MPL equivalent of a function object that returns inherit for arguments T1 and T2.

Also, mpl::vector has a max size limit I'm sure. Right now we have a very minimal set of packet types, but in the future we have to consider the possibility that 100+ packet types will exist (the final number is ultimately unknown). If this is true, I doubt mpl::vector would be usable anymore since 100 seems too large for it. This makes me feel like I'm going to have to resort to a completely template-less approach. I might have to send a generic Packet object to all subscribers, which has a ::Get() method that performs a static_cast<>() to the concrete type based on an ID.

MPL allows configuration to make vector able to hold larger numbers of types. If you need such a large number of Packets, compilation may be very slow with metaprogramming. In Christ, Steven Watanabe

On Wed, Mar 26, 2008 at 2:34 PM, Robert Dailey wrote:

It's good to know that mpl::vector can be configured, however the slow compilation time will require me to ensure that the mpl::vector remains in an implementation file as to not affect the compile time of other files (I'm assuming this is the solution).

At this point I think the very last thing I need is to understand the inherit concepts. I'm still really confused about how they work. In both the doxygen documentation for inherit_linearly and in your examples, I do not see anyone explicitly filling in the '_' arguments. However, the boost examples use _1 and _2 which makes it even more confusing. I'll outline the very specific questions I have:

1) I've never used the _ placeholder, so I'm not really sure how it works. How is it different from _1, _2, etc. Right now I'm familiar with using Boost.Bind with the _1 concepts, how would _ be used in boost.bind (just as an example so I can see the differences).

2) What is the purpose in how mpl::inherit forms its classes? You say it does "class inherit : T1, T2", however I do not see the benefit in this.

3) I understand that mpl::inherit_linearly treats the second template parameter as a binary lambda as you explained, however I was expecting to see that we explicitly fill in the placeholders, and that's the part that's throwing me off. I don't see how the placeholders are being filled.

4) So mpl::inherit_linearly::type is the very base type in the inheritance chain? Does this allow us to access any type simply by performing up-casts? It looks like from the boost docs that the up-cast is performed by assigning the mpl::inherit_linearly::type directly to the type you want, is this correct?

5) Can you give an example of what this inheritance tree looks like generated by mpl::inherit_linearly?

6) Can you give me a small [pseudo]code example of what mpl::inherit_linearly does for each object in the list specified at the first template parameter? The way it internally works conceptually is unknown to me.

7) The explanation you provided (quoted below) makes no sense to me. Could you be so kind as to emphasize a little more? Keep in mind that I have no experience with std::accumulate.

mpl::inherit_linearly treats the second argument as a binary lambda

...

expression. It is very similar to the runtime std::accumulate. Think of inherit<_, _> as the MPL equivalent of a function object that returns inherit for arguments T1 and T2.

Again, thanks for your help and I'm sorry for being so slow. The MPL library has always proven to be frustrating for me to learn. I do appreciate you helping me out with it though. Perhaps my inexperience with MPL is why I have such a difficult time figuring out designs for some of these issues on my own.

I said doxygen in a few places in my previous post- replace those with boost. Sorry, I just got done reading doxygen documentation when I wrote that last post lol.

On Wed, Mar 26, 2008 at 4:37 PM, Steven Watanabe wrote:

AMDG

Robert Dailey wrote:

...

It's good to know that mpl::vector can be configured, however the slow compilation time will require me to ensure that the mpl::vector remains in an implementation file as to not affect the compile time of other files (I'm assuming this is the solution).

At this point I think the very last thing I need is to understand the inherit concepts. I'm still really confused about how they work. In both the doxygen documentation for inherit_linearly and in your examples, I do not see anyone explicitly filling in the '_' arguments. However, the boost examples use _1 and _2 which makes it even more confusing. I'll outline the very specific questions I have:

1) I've never used the _ placeholder, so I'm not really sure how it works. How is it different from _1, _2, etc. Right now I'm familiar with using Boost.Bind with the _1 concepts, how would _ be used in boost.bind (just as an example so I can see the differences). _ is just syntactic sugar. mpl::inherit<_1, _2> would work just as well. The first occurrence of _ is equivalent to _1 The second occurrence of _ is equivalent to _2. The third occurence of _ is equivalent to _3

...

2) What is the purpose in how mpl::inherit forms its classes? You say it does "class inherit : T1, T2", however I do not see the benefit in this.

The ultimate result is a struct that behaves like:

struct signal_holder : boost::signal boost::signal, boost::signal ... {};

This holds instances of each of the boost::signal types and allows access by upcasting.

...

3) I understand that mpl::inherit_linearly treats the second template parameter as a binary lambda as you explained, however I was expecting to see that we explicitly fill in the placeholders, and that's the part that's throwing me off. I don't see how the placeholders are being filled.

We don't fill in the placeholders. mpl::inherit_linearly does.

...

4) So mpl::inherit_linearly::type is the very base type in the inheritance chain?

mpl::inherit_linearly::type is derived from all the types in the sequence. it is not a base class of anything.

...

Does this allow us to access any type simply by performing up-casts? Yes. It looks like from the boost docs that the up-cast is performed by assigning the mpl::inherit_linearly::type directly to the type you want, is this correct? Yes. Or as in my example code, you can cast to a reference to the type you want.

...

5) Can you give an example of what this inheritance tree looks like generated by mpl::inherit_linearly?

empty_base signal | | +---------------+ | inherit<...> signal | | +--------------+ | inherit<...> signal | | +--------------+ | inherit<...>

...

6) Can you give me a small [pseudo]code example of what mpl::inherit_linearly does for each object in the list specified at the first template parameter? The way it internally works conceptually is unknown to me.

type mpl::inherit_linearly(typelist l, lambda_expression f, type base = empty_base) { type result = base; for each x in l { result = mpl::apply(f, result, x); } return(result); }

mpl::apply does the magic of substituting for placeholders.

In Christ, Steven Watanabe

After investigating this a little more (plus from the answers you provided above) I think I fully understand the problem. My only concern at this point is compile time, as you pointed out. For 100 packet types, do you think compile time would be noticeably affected (assuming that all of the boost includes are in a precompiled header file)? How about for 1000 packet types? Thanks so much for your help, you definitely provided me a great design. Perhaps in the future I'll be able to generate great designs like this since now I have learned a lot more about the mpl library.

On Wed, Mar 26, 2008 at 9:20 PM, Steven Watanabe wrote:

AMDG

Robert Dailey wrote:

...

After investigating this a little more (plus from the answers you provided above) I think I fully understand the problem. My only concern at this point is compile time, as you pointed out. For 100 packet types, do you think compile time would be noticeably affected (assuming that all of the boost includes are in a precompiled header file)? How about for 1000 packet types?

1000 packet types is unfeasible because mpl uses the preprocessor lib to generate the vector specializations--Boost.Preprecessor has a hard limit of 256 and some preprocessors give out even before that point.

It turns out that MPL does not actually permit the upper limit of 50 to be configured. This a little annoying especially because all the framework needed is already in place. I had to use a private header to generate vector100 for a test (It took 43 seconds to compile, BTW)

If you have so many types of packets, you are probably better off using runtime dispatching.

typedef boost::unordered_map > dispatcher;

and then store references to the individual signals along with the downcasting logic in the boost::function<>s

In Christ, Steven Watanabe

It's sad to find out about the restrictions of the templates, but I can understand. I just finally began to understand your design proposal and had already planned on using it :( Your unordered_map seems redundant, in that you have packet ID's to different function objects with exactly the same signature. I guess this is so we only dispatch packets to subscribers interested in that specific packet. Could you emphasize a little more on what you mean by "store references to the individual signals"? The downcasting logic could be done via a 'get' method.... packet::get<WalkPacket>(), but this is pretty much exactly like doing static_cast<>() directly. get() would have the potential of returning a NULL auto_ptr if the type specified by the get<>() does not represent the concrete type of the packet.

On Thu, Mar 27, 2008 at 11:30 AM, Steven Watanabe wrote:

AMDG

Robert Dailey wrote:

...

It's sad to find out about the restrictions of the templates, but I can understand. I just finally began to understand your design proposal and had already planned on using it :(

Well, you could use MPL list which does not have a fixed upper limit, but you may just run into compiler limits instead of MPL limits.

...

Your unordered_map seems redundant, in that you have packet ID's to different function objects with exactly the same signature. I guess this is so we only dispatch packets to subscribers interested in that specific packet. Could you emphasize a little more on what you mean by "store references to the individual signals"? The downcasting logic could be done via a 'get' method.... packet::get<WalkPacket>(), but this is pretty much exactly like doing static_cast<>() directly. get() would have the potential of returning a NULL auto_ptr if the type specified by the get<>() does not represent the concrete type of the packet.

typedef boost::unordered_map > dispatcher;

template<class T> struct StaticCaster { StaticCaster(boost::signal &signal) : signal(signal) {} boost::signal &signal; void operator()(Packet const& packet) const { signal(static_cast(packet)); } };

// load the table. // you will probably need a second map to allow functions to // be registered with the signals.

boost::signal walkPacketSignal; dispatcher.insert(make_pair(PID_WALKPACKET, staticCaster<WalkPacket>(walkPacketSignal)));

// dispatching

dispatcher[id](packet);

In Christ, Steven Watanabe

I didn't find any documentation on unordered_map in the boost documentation. What's the reason for not sorting the map? It seems like it would be more inefficient. STL Map is sorted for a reason. BTW thank you for the design here. Can't believe I didn't think about this :)

On 27/03/2008, Scott McMurray wrote:

std::tr1::unordered_map, probably available in boost.tr1 in the next release, implemented as a hash table.

It won't be in 1.35 but will probably be in 1.36.

On Thu, Mar 27, 2008 at 2:50 PM, Robert Dailey wrote:

Thanks for the information guys. Is there any documentation for boost::unordered_map? I can't seem to find it. A link would be greatly appreciated.

http://www.ddj.com/cpp/184402066 might interest you.

On Thu, Mar 27, 2008 at 11:30 AM, Steven Watanabe wrote:

AMDG

Robert Dailey wrote:

...

It's sad to find out about the restrictions of the templates, but I can understand. I just finally began to understand your design proposal and had already planned on using it :(

Well, you could use MPL list which does not have a fixed upper limit, but you may just run into compiler limits instead of MPL limits.

...

Your unordered_map seems redundant, in that you have packet ID's to different function objects with exactly the same signature. I guess this is so we only dispatch packets to subscribers interested in that specific packet. Could you emphasize a little more on what you mean by "store references to the individual signals"? The downcasting logic could be done via a 'get' method.... packet::get<WalkPacket>(), but this is pretty much exactly like doing static_cast<>() directly. get() would have the potential of returning a NULL auto_ptr if the type specified by the get<>() does not represent the concrete type of the packet.

typedef boost::unordered_map > dispatcher;

template<class T> struct StaticCaster { StaticCaster(boost::signal &signal) : signal(signal) {} boost::signal &signal; void operator()(Packet const& packet) const { signal(static_cast(packet)); } };

// load the table. // you will probably need a second map to allow functions to // be registered with the signals.

boost::signal walkPacketSignal; dispatcher.insert(make_pair(PID_WALKPACKET, staticCaster<WalkPacket>(walkPacketSignal)));

// dispatching

dispatcher[id](packet);

Subscription doesn't seem as simple as you proposed. I looked into possible designs for subscribing to the signals and nothing is working out. There's no way to generate a second map to provide slot connections, since the signal object is actually owned by the first map, and thus the two cannot share them. Secondly, the slots (given the design above) each have a different signature for each packet, which further complicates things. Any suggestions? I'm having trouble thinking outside the box...

On Fri, Mar 28, 2008 at 1:03 PM, Steven Watanabe wrote:

AMDG

Robert Dailey wrote:

...

Subscription doesn't seem as simple as you proposed. I looked into possible designs for subscribing to the signals and nothing is working out. There's no way to generate a second map to provide slot connections, since the signal object is actually owned by the first map, and thus the two cannot share them. Secondly, the slots (given the design above) each have a different signature for each packet, which further complicates things.

Any suggestions? I'm having trouble thinking outside the box...

#include #include #include #include #include #include <vector> #include <utility> #include <iostream>

struct Packet {};

struct WalkPacket : Packet {};

enum PacketID { PID_WALKPACKET };

template<class T> struct GetPacketID;

template<> struct GetPacketID<WalkPacket> : boost::mpl::integral_c {};

template<class T> struct StaticCaster { StaticCaster(boost::shared_ptr > const& signal) : signal(signal) {} boost::shared_ptr > signal; void operator()(Packet const& packet) const { (*signal)(static_cast(packet)); } };

...

(iter->second.first)->connect(f); }; void operator()(PacketID id, const Packet& packet) { dispatcher[id].second(packet); }

class SignalHolder { public: template void register_function(F f) { const PacketID id = GetPacketID<T>::value; dispatcher_t::iterator iter = dispatcher.find(id); if(iter == dispatcher.end()) { boost::shared_ptr > signal(new boost::signal()); iter = dispatcher.insert(std::make_pair(id, std::make_pair(static_cast(signal), StaticCaster<T>(signal)))).first; } boost::static_pointer_cast private: typedef boost::unordered_map

...

...

dispatcher_t; dispatcher_t dispatcher; };

void test(WalkPacket const&) { std::cout << "Got a WalkPacket" << std::endl; }

int main() {

SignalHolder holder;

holder.register_function<WalkPacket>(&test);

WalkPacket packet;

holder(PID_WALKPACKET, packet); }

In Christ, Steven Watanabe

Thanks, this is a great idea. However, while you're doing the insert() I noticed you're using statc_cast<>(). I'm actually surprised this compiles. This is basically like casting something to an unrelated type, so I'm not sure how the compiler is allowing this behavior. This also seems very inconsistent in that you're using boost::static_pointer_cast() for going from basically a void* to a signal object.

On Fri, Mar 28, 2008 at 3:49 PM, Steven Watanabe wrote:

AMDG

...

Thanks, this is a great idea. However, while you're doing the insert() I noticed you're using statc_cast<>(). I'm actually surprised this compiles.

Robert Dailey wrote: the static_cast calls the templated shared_ptr constructor.

...

This is basically like casting something to an unrelated type, so I'm not sure how the compiler is allowing this behavior. This also seems very inconsistent in that you're using boost::static_pointer_cast() for going from basically a void* to a signal object.

Fine. Consistency wouldn't hurt.

Sorry if it seemed like I was picking on you. I actually meant to ask why you chose one or the other. Thanks for the info.

On Fri, Mar 28, 2008 at 4:10 PM, Asif Lodhi wrote:

Hi Robert,

On 3/28/08, Robert Dailey wrote:

...

On Thu, Mar 27, 2008 at 11:30 AM, Steven Watanabe wrote: X ............................................................................................................. Any suggestions? I'm having trouble thinking outside the box...

Don't you think that _downcasting_ the return value of your factory method defeats the whole point of using the factory method in the first place? It looks like that you need to use "parameterized" factories so that when you read the packet-id, your factory method instantiates an object of the correct packet class after reading the packet-id. As regards to sending the packet, it looks like you need to have some kind of a "dispatcher" class in place which should have a send method taking a reference of your base packet class as a parameter. Don't you think it would solve the downcasting problem? If you do have some functionality in your the child packet classes that is forcing you to downcast then may be you should rethink your object-model design.

Don't you think your problem is a case for re-design instead of using Boost?

Just my two cents.

The problem is that the type of the packet is not known until runtime. This makes it very difficult to make things "type safe" at compile time. What Steven has presented is about as type-safe as you can make it. However, the system does not prevent you from mismatching ID's with actual packet types, but that's just a natural problem with data driven designs. Right now there is no possible way to have a factory that returns the original type of the packet, since what is being passed in (the ID) is not known at compile time, as I've already stated. With all of Steven's excellent help and some thought process of my own I am unable to find a better design than the one I have. It's not fair to compare runtime-designs with compile-time designs. Thanks for your input.

Hi Robert, On 3/29/08, Steven Watanabe wrote:

AMDG

Robert Dailey wrote:

...

The problem is that the type of the packet is not known until runtime. This makes it very difficult to make things "type safe" at compile time. What Steven has presented is about as type-safe as you can make it. However, the system does not prevent you from mismatching ID's with actual packet types, but that's just a natural problem with data driven designs. You could put the ID in the base Packet type. That should minimize the chances of mismatch.

YES.

...

Right now there is no possible way to have a factory that returns the original type of the packet, since what is being passed in (the ID) is not known at compile time, as I've already stated. It seems like a problem in multiple dispatching, which C++ just doesn't support very well natively.

Don't you think you could have a protected data member (packet_type, for example) for storing the type ID of the packet in the base class AND a public virtual function in the base class (subsquently overridden in all the derived packet classes) to return the ACTUAL type "ID of the packet (if that's what you want)? This will give you the type ID of the actual type of packet simply and easily. As I have said before, you CAN do it using a simple class/object model and simple application of patterns. -- Best regards, Asif

On Fri, Mar 28, 2008 at 7:26 PM, Steven Watanabe wrote:

AMDG

Robert Dailey wrote:

...

The problem is that the type of the packet is not known until runtime. This makes it very difficult to make things "type safe" at compile time. What Steven has presented is about as type-safe as you can make it. However, the system does not prevent you from mismatching ID's with actual packet types, but that's just a natural problem with data driven designs. You could put the ID in the base Packet type. That should minimize the chances of mismatch.

Right now I'm already putting the ID as a static constant variable in each derived type. I don't see a way to set the ID in the base Packet class, since that would more than likely require CRTP or something. From what I have seen the Packet base class cannot be a template. I suppose you could do this if you used 3 levels of inheritance, but that seems ugly.

original type of the packet, since what is being passed in (the ID) is

...

not known at compile time, as I've already stated. Right now there is no possible way to have a factory that returns the It seems like a problem in multiple dispatching, which C++ just doesn't support very well natively.

In Christ, Steven Watanabe

On Sat, Mar 29, 2008 at 12:08 PM, Steven Watanabe wrote:

AMDG

Robert Dailey wrote:

...

You could put the ID in the base Packet type. That should minimize the chances of mismatch.

Right now I'm already putting the ID as a static constant variable in each derived type. I don't see a way to set the ID in the base Packet class, since that would more than likely require CRTP or something. From what I have seen the Packet base class cannot be a template. I suppose you could do this if you used 3 levels of inheritance, but that seems ugly.

I'm thinking of a non static member variable in the base class, which can be set in the constructor.

In Christ, Steven Watanabe

Is there any reason why the ID needs to be available at runtime? Instead of your specialization, I do: class WalkPacket : Packet { static const PacketId ID = PID_WALKPACKET; }; Is there any reason why this isn't sufficient?

On Sat, Mar 29, 2008 at 3:21 PM, Steven Watanabe wrote:

AMDG

Robert Dailey wrote:

...

Is there any reason why the ID needs to be available at runtime? Instead of your specialization, I do:

class WalkPacket : Packet { static const PacketId ID = PID_WALKPACKET; };

Is there any reason why this isn't sufficient?

The dispatcher needs to know what the ID of a packet is to do its job. The idea is that instead of passing the ID and the packet separately, the packet knows its own ID. It is possible, though to replace the ID type with std::string in the dispatcher and use std::string(typeid(packet).name()) avoiding the ID's entirely inside the dispatcher.

It seems that so far we've gotten everything done without RTTI, I would hate to introduce it here. I see what you're saying now. Something like this might be in order: struct Packet { virtual PacketId GetId() const = 0; }; struct WalkPacket : Packet { static const PacketId ID = PID_WALKPACKET; virtual PacketId GetId() const { return ID; } };

On Sat, Mar 29, 2008 at 3:40 PM, Steven Watanabe wrote:

AMDG

Robert Dailey wrote:

...

It seems that so far we've gotten everything done without RTTI, I would hate to introduce it here.

RTTI is not always evil. It may be less efficient to hash the strings, but it is not worse from a design perspective.

...

I see what you're saying now. Something like this might be in order:

struct Packet { virtual PacketId GetId() const = 0; };

struct WalkPacket : Packet { static const PacketId ID = PID_WALKPACKET; virtual PacketId GetId() const { return ID; } };

Yep. That works. Just thought I'd point out that this is not so very different from RTTI, after all.

In Christ, Steven Watanabe

Well I'm not 100% sure of the performance impact RTTI will have on the application. What I've read is that RTTI can get very expensive in terms of executable size if you have a lot of classes in your application. Unfortunately I don't think I could get away with using strings since the network is assigning integral values to the packets in order to uniquely identify them. I like the string idea because it doesn't have the repetitive code in each derived class (the overridden GetID method that will be the same in each derived class).

On Sat, Mar 29, 2008 at 3:58 PM, Steven Watanabe wrote:

AMDG

Robert Dailey wrote:

...

Well I'm not 100% sure of the performance impact RTTI will have on the application. What I've read is that RTTI can get very expensive in terms of executable size if you have a lot of classes in your application.

You should be able to switch between using RTTI and using ID's in the dispatcher with a few lines of code. *Measure the difference* before making decisions for performance considerations.

...

Unfortunately I don't think I could get away with using strings since the network is assigning integral values to the packets in order to uniquely identify them. Of course, if you need the ID's around anyway for other reasons, they are not a problem. I wouldn't expend Herculean efforts to eliminate them.

...

I like the string idea because it doesn't have the repetitive code in each derived class (the overridden GetID method that will be the same in each derived class).

struct WalkPacket : CRTPPacket {}; ?

What exactly is this structure supposed to present?

On Sat, Mar 29, 2008 at 7:06 PM, Steven Watanabe wrote:

AMDG

Robert Dailey wrote:

...

struct WalkPacket : CRTPPacket {}; ?

What exactly is this structure supposed to present?

I was just thinking of moving the duplicated code into CRTPPacket

template struct CRTPPacket : Packet { static const PacketID ID = Id; virtual PacketID getID() const { return(ID); } };

In Christ, Steven Watanabe

That's exactly what I was thinking :) I just wanted to make sure!

@Nat What you have provided is the exact and final implementation I chose. I found no reason to pass the derived type to the base. @Steven I never took the time to say thank you. You've been absolutely wonderful, I really do appreciate all of the help you've given me. I almost feel like I should pay you... but don't get any smart ideas about that! :) You take care and thanks so much. On Wed, Apr 2, 2008 at 9:54 AM, Nat Goodspeed wrote:

Steven Watanabe wrote:

...

I was just thinking of moving the duplicated code into CRTPPacket

template struct CRTPPacket : Packet { static const PacketID ID = Id; virtual PacketID getID() const { return(ID); } };

Does it even need to be CRTP?

template<PacketID Id> struct IDPacket : Packet { static const PacketID ID = Id; virtual PacketID getID() const { return(ID); } };

Perhaps we're assuming other methods that would make use of Derived. _______________________________________________ Boost-users mailing list Boost-users@lists.boost.org http://lists.boost.org/mailman/listinfo.cgi/boost-users

Hi Robert, On 3/29/08, Robert Dailey wrote:

On Fri, Mar 28, 2008 at 4:10 PM, Asif Lodhi wrote: X...............................................................................................Right now there is no possible way to have a factory that returns the original type of the packet, since what is being passed in (the ID) is not known at compile time, as I've already stated.

I have just re-read the whole thread. Downcasting the return value of a factory method really seems strange to me. I think, here is what you need to do: 1) Create a protected "packet_id" data member in your base "Packet" class, as Steven said earlier. 2) Create an abstract virtual function in your base class - for example: virtual const int &PacketTypeID()=0; // NO NEED to try finding a factory method that returns the original type of the // packet. The above virtual function and its over-ridden variants in each child // packet class will correctly return the original packet type - that is, the "ID" integer // of the packet - if that's what you mean by the original packet type. 3) In your factory method, add code to store the "ID" integer of the packet received in the packet_id protected data member. 4) Over-ride the virtual function specified in (2) in each child class of Packet. and just return the packet_id. 5) Use a simple template-based Observer to implement event dispatch instead of using a look-up based function pointer mechanism - no need to use MPL. 5) NO NEED TO DOWNCAST the return value of your factory method. Your factory method should have a reference/pointer to the base Packet class as the return type. Just store this return value in a variable of the "base Packet type" instead of downcasting to the concrete PakcetType. If you store the reference/pointer of the base Packet type (that your factory method returns) in a variable of the BASE PacketType (which will actuall a reference to the concrete Packet type if your factory method is really a factory method) then the built-in polymorphism will correctly call the implementation of the PacketTypeID() function defined in the corresponding concrete child Packet class. 6) Instead of a look-up table of function pointers you should define a virtual function for signal dispatch in the base class and over-ride the same in each child Packet class to enable execution of the correct code. 7) If there are too many Packet types then you can do the whole thing using SIMPLE templates, instead of resorting to MPL, so that the appropriate concrete instances of each concrete [child] Packet class (and their virtual functions for returning the packet_id and dispatching) automatically get generated at compile time. You said that the packet type ID is not known at compile time - I wonder how, then, you are mapping IDs with the Packet types. Your reply seems to suggest that, each time you get a certain type of Packet, it can have a different ID. If that's not the case then I think the above strategy will do. -- Best regards, Asif

Hi Steven, On 3/29/08, Steven Watanabe wrote:

X................................. Using both a function and a variable is redundant. Either have a virtual function that can be overridden to specify the ID or a constructor parameter and a variable. Not both.

Yes. It was a mistake - I scribbled the whole thing very hurriedly.

There are already function pointers in Boost.Signals. I'm pretty sure that this observer requires either the use of global signals (Yuck) or a cyclic visitor (Also not nice). Using a map achieves the same effect, keeping the dispatching logic cleanly separated from the individual handlers and from the event objects. Incidentally, if you look at my last rendition, there is no MPL involved.

It all depends on your design - as to how you implement the observer pattern - spraying global variables all over the place or using proper encapsulation. I am not much into Boost Signals but if it results in clean separation then one should go for it. I just suggested a way to implement the whole thing using simple C++ & basic design patterns without any external library.

IMO, this is just another means of downcasting. It's safer than a raw static cast because the types will be checked automatically, but not any better than the map solution (Which encapsulates the type check and downcast).

Well, IMO, polymorphically invoking virtual functions is better than downcasting.

This of course is the traditional OO wisdom, but IMO, maps are a perfectly legitimate means of dispatching.

If it results in significant performance gains then, of course, yes. But you cannot know that until you actually measure the performance. Design wise, I'd prefer the OO approach. Since I have just read the rest of the thread, I would like to add that Boost ASIO and ACE both are designed for this kind of communication stuff. I don't know about ASIO but I do know that ACE does use RTTI, virtual functions and design patterns. I don't think it makes any significant performance difference. As a matter of fact, using templates would often result in faster code than using straight function pointers. A better strategy for Robert would be to actually measure it or use one of Boost ASIO or ACE libraries. -- Best regards, Asif

Hi Steven, On 3/30/08, Steven Watanabe wrote:

......................................................... There needs to be a way to register callbacks for each particular event type. Then, there is some code that gets a packet and decodes it to the proper type. Finally, this is dispatched to all the registered signals.

That's exactly why I suggested using the Observer design pattern. -- BR/Asif

On Fri, Mar 28, 2008 at 3:49 PM, Steven Watanabe wrote:

AMDG

...

Thanks, this is a great idea. However, while you're doing the insert() I noticed you're using statc_cast<>(). I'm actually surprised this compiles.

Robert Dailey wrote: the static_cast calls the templated shared_ptr constructor.

...

This is basically like casting something to an unrelated type, so I'm not sure how the compiler is allowing this behavior. This also seems very inconsistent in that you're using boost::static_pointer_cast() for going from basically a void* to a signal object.

Fine. Consistency wouldn't hurt.

In Christ, Steven Watanabe

@Steven I find it really amazing that you come up with solutions so quickly. They're great design suggestions. Do you have any advice for me so that in the future I may be able to come up with designs like this? Do I just need to read the whole boost::mpl library documentation or something? Thanks.