Need boost's help with poor design
Hi, Currently I'm using Boost.Signals to transfer Packet objects to subscribers. Right now my Boost.Signal looks like below: boost::signal<void (Packet const& p)> 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<WalkPacket const&>( 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. Keep this important note in mind: The type of the packet that will be dispatched to subscribers can only be known at compile time via a factory method. Data is received from the network, and that data is packed into the correct packet and returned as a Packet* base class pointer, so that we can transfer the data to subscribers through a common interface.
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<void (Packet const& p)>
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<WalkPacket const&>( 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<class F, class T> 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<arg_type*>(&u)) { f(*arg); } } F f; }; template<class T, class F> downcaster<F, T> make_downcaster(F f) { return(downcaster<F, T>(f)); } In Christ, Steven Watanabe
On Tue, Mar 25, 2008 at 4:41 PM, Steven Watanabe <watanabesj@gmail.com> 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<void (Packet const& p)>
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<WalkPacket const&>( 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<class F, class T> 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<arg_type*>(&u)) { f(*arg); } } F f; }; template<class T, class F> downcaster<F, T> make_downcaster(F f) { return(downcaster<F, T>(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<PID_WALKPACKET, boost::signal<void (WalkPacket const&)> >, pair<PID_CHATPACKET, boost::signal<void (ChatPacket const&)> >, pair<PID_QUESTPACKET, boost::signal<void (QuestPacket const&)> >
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.
AMDG Robert Dailey wrote:
template<class F, class T> 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<arg_type*>(&u)) { f(*arg); } } F f; }; template<class T, class F> downcaster<F, T> make_downcaster(F f) { return(downcaster<F, T>(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.
The type of the signal would be boost::signal<void (Packet const& p)> When you register a function object with the signal, you would say: make_downcaster<WalkPacket>(f) to get a function object that will a) call f(x) if x is a WalkPacket b) do nothing if x is something else. I believe that this does in fact solve your problem but will be horribly inefficient, since it repeats the test for every function object.
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<PID_WALKPACKET, boost::signal<void (WalkPacket const&)> >, pair<PID_CHATPACKET, boost::signal<void (ChatPacket const&)> >, pair<PID_QUESTPACKET, boost::signal<void (QuestPacket const&)> >
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?
Not to my knowledge. There are several approaches. You can of course hard code the switch. If there are not a lot of different packet types this may the easiest alternative. #include <boost/signal.hpp> enum PacketIDs { PID_WALKPACKET = 0, PID_CHATPACKET, PID_QUESTPACKET }; struct Packet {}; struct WalkPacket : Packet {}; struct ChatPacket : Packet {}; struct QuestPacket : Packet {}; struct SignalHolder { boost::signal<void (WalkPacket const&)> m_sigWalk; boost::signal<void (ChatPacket const&)> m_sigChat; boost::signal<void (QuestPacket const&)> m_sigQuest; void dispatch(PacketIDs id, Packet const& packet) { switch(id) { case PID_WALKPACKET: m_sigWalk(static_cast<WalkPacket const&>(packet)); break; case PID_CHATPACKET: m_sigChat(static_cast<ChatPacket const&>(packet)); break; case PID_QUESTPACKET: m_sigQuest(static_cast<QuestPacket const&>(packet)); break; } } }; It's possible to use the preprocessor to "automate" this but that's extremely ugly. You might also look into replacing Packet with boost::variant.
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.
No it won't as you said above.
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.
In Christ, Steven Watanabe
Steven Watanabe wrote:
You might also look into replacing Packet with boost::variant.
I would second this. If you use: typedef boost::variant<WalkPacket,CharPacket,etc> 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<WalkPacket,ChatPacket,etc> Packet; // Your signals... boost::signal<void (WalkerPacket)> distributeWalkerPacket; boost::signal<void (ChatPacket)> 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
On Wed, Mar 26, 2008 at 2:03 AM, Kevin Scarr <kscarr@iinet.net.au> wrote:
Steven Watanabe wrote:
You might also look into replacing Packet with boost::variant.
I would second this. If you use:
typedef boost::variant<WalkPacket,CharPacket,etc> 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<WalkPacket,ChatPacket,etc> Packet;
// Your signals... boost::signal<void (WalkerPacket)> distributeWalkerPacket; boost::signal<void (ChatPacket)> 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
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. I couldn't put them in a std::map, since the value type is always required to be the same. I could use boost::tuple if I could guarantee that my packet ID's started at 0 and where contiguous thereafter (however the size limit of a tuple would be too small). Something like a tuple, except as a map would be a great solution. However since someone has already pretty much addressed that no such container exists, I'm still looking for clean alternatives. Consider that I may have 100 packet types (which is not unreasonable in the game we're working on), it would quickly become unmanageable to have 100 signal variables. Perhaps having 1 signal for all packets might be a good alternative, however I'd have to sit down and think about how it is going to work. Using 1 signal for all packets, as I pointed out before, really constrains the signature of the slots. Perhaps I should just send the boost::variant to my slots? Thanks everyone for your continued help. You're really giving me some great ideas.
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<WalkPacket, ChatPacket, QuestPacket> packet_types; typedef boost::mpl::transform<boost::signal1<void, _> >::type signals; typedef boost::mpl::inherit_linearly<signals, static_visitor<>, mpl::inherit<_, _> >::type SignalVisitor; In Christ, Steven Watanabe
On Wed, Mar 26, 2008 at 9:53 AM, Steven Watanabe <watanabesj@gmail.com> wrote:
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<WalkPacket, ChatPacket, QuestPacket> packet_types; typedef boost::mpl::transform<boost::signal1<void, _> >::type signals; typedef boost::mpl::inherit_linearly<signals, static_visitor<>, mpl::inherit<_, _> >::type SignalVisitor;
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?
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 <boost/mpl/vector.hpp> #include <boost/mpl/transform.hpp> #include <boost/mpl/inherit_linearly.hpp> #include <boost/mpl/inherit.hpp> #include <boost/signal.hpp> #include <boost/variant.hpp> 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<void(T const&)> type; }; // a type list of all the possible packet types. typedef mpl::vector<WalkPacket, ChatPacket, QuestPacket> 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<void(WalkPacket const&)>, signal<void(ChatPacket const&)>, signal<void(QuestPacket const&)> > // // 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<make_signal<_>, T> // will replace _ with T and then return the nested ::type // typename make_signal<T>::type. typedef mpl::transform<packet_types, make_signal<_> >::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<empty_base, WalkPacket>. // Then, it continues with the second element of signals // to get inherit<inherit<empty_base, WalkPacket>, 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<signals, mpl::inherit<_, _> >::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<typename make_signal<T>::type&>(holder_)(t)); } }; typedef boost::make_variant_over<packet_types>::type packet; int main() { packet p = WalkPacket(); signal_holder signals; boost::apply_visitor(signal_visitor(signals), p); } In Christ, Steven Watanabe
On Wed, Mar 26, 2008 at 11:33 AM, Steven Watanabe <watanabesj@gmail.com> wrote:
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.
<snip> In Christ, Steven Watanabe
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<empty_base, WalkPacket> what exactly is that doing? 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? 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. 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. The only reason I haven't decided to use your ideas so far (which are extremely great by the way) is because of the limitations of the design. The design imposes a limit on the number of packet types we can support, which isn't acceptable given the requirements. Thanks again for your help.
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<empty_base, WalkPacket> what exactly is that doing?
template<class T1, class T2> 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<T1, T2> 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 1:36 PM, Steven Watanabe <watanabesj@gmail.com> wrote:
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<empty_base, WalkPacket> what exactly is that doing?
template<class T1, class T2> 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<T1, T2> 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.
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<T1, T2> 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.
On Wed, Mar 26, 2008 at 2:34 PM, Robert Dailey <rcdailey@gmail.com> 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<T1, T2> 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.
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<void(WalkPacket const&)> boost::signal<void(ChatPacket const&)>, boost::signal<void(QuestPacket const&)> ... {}; 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<void(WalkPacket)> | | +---------------+ | inherit<...> signal<void(ChatPacket)> | | +--------------+ | inherit<...> signal<void(QuestPacket)> | | +--------------+ | 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
On Wed, Mar 26, 2008 at 4:37 PM, Steven Watanabe <watanabesj@gmail.com> 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<void(WalkPacket const&)> boost::signal<void(ChatPacket const&)>, boost::signal<void(QuestPacket const&)> ... {};
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<void(WalkPacket)> | | +---------------+ | inherit<...> signal<void(ChatPacket)> | | +--------------+ | inherit<...> signal<void(QuestPacket)> | | +--------------+ | 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.
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<PacketID, boost::function<void(Packet const&)> > dispatcher; and then store references to the individual signals along with the downcasting logic in the boost::function<>s In Christ, Steven Watanabe
On Wed, Mar 26, 2008 at 9:20 PM, Steven Watanabe <watanabesj@gmail.com> 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<PacketID, boost::function<void(Packet const&)> > 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.
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<PacketID, boost::function<void(Packet const&)> > dispatcher; template<class T> struct StaticCaster { StaticCaster(boost::signal<void(T const&)> &signal) : signal(signal) {} boost::signal<void(T const&)> &signal; void operator()(Packet const& packet) const { signal(static_cast<T const&>(packet)); } }; // load the table. // you will probably need a second map to allow functions to // be registered with the signals. boost::signal<void(WalkPacket const&)> walkPacketSignal; dispatcher.insert(make_pair(PID_WALKPACKET, staticCaster<WalkPacket>(walkPacketSignal))); // dispatching dispatcher[id](packet); In Christ, Steven Watanabe
On Thu, Mar 27, 2008 at 11:30 AM, Steven Watanabe <watanabesj@gmail.com> 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<PacketID, boost::function<void(Packet const&)> > dispatcher;
template<class T> struct StaticCaster { StaticCaster(boost::signal<void(T const&)> &signal) : signal(signal) {} boost::signal<void(T const&)> &signal; void operator()(Packet const& packet) const { signal(static_cast<T const&>(packet)); } };
// load the table. // you will probably need a second map to allow functions to // be registered with the signals.
boost::signal<void(WalkPacket const&)> 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 Thu, Mar 27, 2008 at 2:21 PM, Robert Dailey <rcdailey@gmail.com> wrote:
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.
std::tr1::unordered_map, probably available in boost.tr1 in the next release, implemented as a hash table.
On Thu, Mar 27, 2008 at 1:30 PM, Daniel James <daniel_james@fmail.co.uk> wrote:
On 27/03/2008, Scott McMurray <me22.ca+boost@gmail.com> 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.
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.
On Thu, Mar 27, 2008 at 2:50 PM, Robert Dailey <rcdailey@gmail.com> 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 27/03/2008, Scott McMurray <me22.ca+boost@gmail.com> wrote:
On Thu, Mar 27, 2008 at 2:50 PM, Robert Dailey <rcdailey@gmail.com> 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.
The work in progress documentation is at: http://unordered.nfshost.com/doc/html/unordered.html
http://www.ddj.com/cpp/184402066 might interest you.
I should probably link to this somewhere. It's a pity the follow up article wasn't published. Daniel
On Thu, Mar 27, 2008 at 11:30 AM, Steven Watanabe <watanabesj@gmail.com> 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<PacketID, boost::function<void(Packet const&)> > dispatcher;
template<class T> struct StaticCaster { StaticCaster(boost::signal<void(T const&)> &signal) : signal(signal) {} boost::signal<void(T const&)> &signal; void operator()(Packet const& packet) const { signal(static_cast<T const&>(packet)); } };
// load the table. // you will probably need a second map to allow functions to // be registered with the signals.
boost::signal<void(WalkPacket const&)> 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...
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...
(iter->second.first)->connect(f); }; void operator()(PacketID id, const Packet& packet) { dispatcher[id].second(packet); }
#include <boost/signal.hpp> #include <boost/function.hpp> #include <boost/unordered_map.hpp> #include <boost/shared_ptr.hpp> #include <boost/mpl/integral_c.hpp> #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<PacketID, PID_WALKPACKET> {}; template<class T> struct StaticCaster { StaticCaster(boost::shared_ptr<boost::signal<void(T const&)> > const& signal) : signal(signal) {} boost::shared_ptr<boost::signal<void(T const&)> > signal; void operator()(Packet const& packet) const { (*signal)(static_cast<T const&>(packet)); } }; class SignalHolder { public: template<class T, class F> 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<boost::signal<void(T const&)> > signal(new boost::signal<void(T const&)>()); iter = dispatcher.insert(std::make_pair(id, std::make_pair(static_cast<boost::shared_ptr<void> >(signal), StaticCaster<T>(signal)))).first; } boost::static_pointer_cast<boost::signal<void(T const&)> private: typedef boost::unordered_map<PacketID, std::pair<boost::shared_ptr<void>, boost::function<void(Packet const&)>
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
On Fri, Mar 28, 2008 at 1:03 PM, Steven Watanabe <watanabesj@gmail.com> 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 <boost/signal.hpp> #include <boost/function.hpp> #include <boost/unordered_map.hpp> #include <boost/shared_ptr.hpp> #include <boost/mpl/integral_c.hpp> #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<PacketID, PID_WALKPACKET> {};
template<class T> struct StaticCaster { StaticCaster(boost::shared_ptr<boost::signal<void(T const&)> > const& signal) : signal(signal) {} boost::shared_ptr<boost::signal<void(T const&)> > signal; void operator()(Packet const& packet) const { (*signal)(static_cast<T const&>(packet)); } };
(iter->second.first)->connect(f); }; void operator()(PacketID id, const Packet& packet) { dispatcher[id].second(packet); }
class SignalHolder { public: template<class T, class F> 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<boost::signal<void(T const&)> > signal(new boost::signal<void(T const&)>()); iter = dispatcher.insert(std::make_pair(id, std::make_pair(static_cast<boost::shared_ptr<void> >(signal), StaticCaster<T>(signal)))).first; } boost::static_pointer_cast<boost::signal<void(T const&)> private: typedef boost::unordered_map<PacketID, std::pair<boost::shared_ptr<void>, boost::function<void(Packet const&)>
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.
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.
AMDG 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
On Fri, Mar 28, 2008 at 3:49 PM, Steven Watanabe <watanabesj@gmail.com> 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.
Hi Robert, On 3/28/08, Robert Dailey <rcdailey@gmail.com> wrote:
On Thu, Mar 27, 2008 at 11:30 AM, Steven Watanabe <watanabesj@gmail.com> 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. -- Best regards, Asif
On Fri, Mar 28, 2008 at 4:10 PM, Asif Lodhi <asif.lodhi@gmail.com> wrote:
Hi Robert,
On 3/28/08, Robert Dailey <rcdailey@gmail.com> wrote:
On Thu, Mar 27, 2008 at 11:30 AM, Steven Watanabe <watanabesj@gmail.com> 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.
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 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.
In Christ, Steven Watanabe
Hi Robert, On 3/29/08, Steven Watanabe <watanabesj@gmail.com> 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 <watanabesj@gmail.com> 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
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
On Sat, Mar 29, 2008 at 12:08 PM, Steven Watanabe <watanabesj@gmail.com> 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?
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. In Christ, Steven Watanabe
On Sat, Mar 29, 2008 at 3:21 PM, Steven Watanabe <watanabesj@gmail.com> 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; } };
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
On Sat, Mar 29, 2008 at 3:40 PM, Steven Watanabe <watanabesj@gmail.com> 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).
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<WalkPacket, PID_WALKPACKET> {}; ? In Christ, Steven Watanabe
On Sat, Mar 29, 2008 at 3:58 PM, Steven Watanabe <watanabesj@gmail.com> 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<WalkPacket, PID_WALKPACKET> {}; ?
What exactly is this structure supposed to present?
AMDG Robert Dailey wrote:
struct WalkPacket : CRTPPacket<WalkPacket, PID_WALKPACKET> {}; ?
What exactly is this structure supposed to present?
I was just thinking of moving the duplicated code into CRTPPacket template<class Derived, PacketID Id> struct CRTPPacket : Packet { static const PacketID ID = Id; virtual PacketID getID() const { return(ID); } }; In Christ, Steven Watanabe
On Sat, Mar 29, 2008 at 7:06 PM, Steven Watanabe <watanabesj@gmail.com> wrote:
AMDG
Robert Dailey wrote:
struct WalkPacket : CRTPPacket<WalkPacket, PID_WALKPACKET> {}; ?
What exactly is this structure supposed to present?
I was just thinking of moving the duplicated code into CRTPPacket
template<class Derived, PacketID Id> 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!
Steven Watanabe wrote:
I was just thinking of moving the duplicated code into CRTPPacket
template<class Derived, PacketID Id> 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.
@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 <nat@lindenlab.com> wrote:
Steven Watanabe wrote:
I was just thinking of moving the duplicated code into CRTPPacket
template<class Derived, PacketID Id> 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 Roberts, On 3/30/08, Robert Dailey <rcdailey@gmail.com> wrote:
.............................................................................. 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; } };
That's exactly what I was trying to say in my earlier reply. -- Best regards, Asif
Hi Robert, On 3/29/08, Robert Dailey <rcdailey@gmail.com> wrote:
On Fri, Mar 28, 2008 at 4:10 PM, Asif Lodhi <asif.lodhi@gmail.com> 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
AMDG Asif Lodhi wrote:
Hi Robert,
On 3/29/08, Robert Dailey <rcdailey@gmail.com> wrote:
On Fri, Mar 28, 2008 at 4:10 PM, Asif Lodhi <asif.lodhi@gmail.com> 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.
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.
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.
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.
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.
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).
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.
This of course is the traditional OO wisdom, but IMO, maps are a perfectly legitimate means of dispatching. In Christ, Steven Watanabe
Hi Steven, On 3/29/08, Steven Watanabe <watanabesj@gmail.com> 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
AMDG Asif Lodhi wrote:
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.
Ok. But I think you missed some details. 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. The cleanest interface I can think of is to have a holder that you can register the callbacks. This is of course avoiding globals out of principle. Then at the other end the dispatcher does some magic to find which signals need to be called. This can be done either through virtual functions or through a table lookup. It doesn't really matter.
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.
As I see it, there is no particular difference in this case. They are two different ways of doing exactly the same thing, and the difference should not be visible to more than about 100 lines of code, max.
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.
My guess is that a virtual function approach will be negligibly faster. In Christ, Steven Watanabe
Hi Steven, On 3/30/08, Steven Watanabe <watanabesj@gmail.com> 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
AMDG Asif Lodhi wrote:
Hi Steven,
On 3/30/08, Steven Watanabe <watanabesj@gmail.com> 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.
How is anything suggested in this thread not an instance of Observer, I might ask? I guess the problem is: where do the callbacks get registered? You can't register them with the event because the event doesn't exist yet. You can create a global object to register for each Packet type, but globals are not usually a good idea. It is also possible to have a separate object for each type of Packet, but they are all tightly bound together and all need to be available when the Packet comes in. This leads to having one class that holds all the callbacks. Now, we could create a separate signal for each Packet type in the class body: class SignalHolder { public: void operator()(const Packet1& packet) const { signal1(packet); } void operator()(const Packet2& packet) const { signal2(packet); } void operator()(const Packet3& packet) const { signal3(packet); } ... void operator()(const PacketN& packet) { signalN(packet); } private: signal<void(Packet1 const&)> signal1; signal<void(Packet2 const&)> signal2; signal<void(Packet3 const&)> signal3; ... signal<void(PacketN const&)> signalN; }; class Packet { virtual void dispatch(const SignalHolder&) = 0; }; class Packet1 : public Packet { virtual void dispatch(const SignalHolder& visitor) { visitor(*this); } }; The only real difference between this and the map solution is that this may be slightly faster and has one more component that needs to be updated when a new packet type is added. If you have a better way of doing this in mind, let me know. In Christ, Steven Watanabe
On Fri, Mar 28, 2008 at 3:49 PM, Steven Watanabe <watanabesj@gmail.com> 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.
Hi Robert, You may not want your Packet clients to have to downcast through Packet subtypes but you can define your Packet subtypes to rely on particular clients, like so (untested, not compiled): class Packet { public: virtual void process( PacketProcessor* p_PacketProcessor ) = 0; }; class WalkPacket : public Packet { public: virtual void process(PacketProcessor* p_PacketProcessor ) { WalkPacketProcessor* l_WalkPacketProcessor = dynamic_cast< WalkPacketProcessor* >( p_PacketProcessor ); if( l_WalkPacketProcessor ) l_WalkPacketProcessor->handleWalkPacket( this ); } }; class PacketProcessor // base class for all PacketProcessors { public: virtual ~PacketProcessor() = 0; // define this in .cpp }; class WalkPacketProcessor : public PacketProcessor { public: // the slot for any packet void handlePacket( Packet const& p ) { p.process( this ); } // the particular handler for WalkPackets virtual void handleWalkPacket( WalkPacket* p_WalkPacket ) { // use this packet } }; Jerry Ferentinos Application Engineering Services, Montreal MRL - IT Merck Frosst Center for Therapeutic Research www.merckfrosstlab.ca ________________________________ From: boost-users-bounces@lists.boost.org [mailto:boost-users-bounces@lists.boost.org] On Behalf Of Robert Dailey Sent: Tuesday, March 25, 2008 5:23 PM To: boost [usr] Subject: [Boost-users] Need boost's help with poor design Hi, Currently I'm using Boost.Signals to transfer Packet objects to subscribers. Right now my Boost.Signal looks like below: boost::signal<void (Packet const& p)> 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<WalkPacket const&>( 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. Keep this important note in mind: The type of the packet that will be dispatched to subscribers can only be known at compile time via a factory method. Data is received from the network, and that data is packed into the correct packet and returned as a Packet* base class pointer, so that we can transfer the data to subscribers through a common interface. ------------------------------------------------------------------------------ Notice: This e-mail message, together with any attachments, contains information of Merck & Co., Inc. (One Merck Drive, Whitehouse Station, New Jersey, USA 08889), and/or its affiliates (which may be known outside the United States as Merck Frosst, Merck Sharp & Dohme or MSD and in Japan, as Banyu - direct contact information for affiliates is available at http://www.merck.com/contact/contacts.html) that may be confidential, proprietary copyrighted and/or legally privileged. It is intended solely for the use of the individual or entity named on this message. If you are not the intended recipient, and have received this message in error, please notify us immediately by reply e-mail and then delete it from your system. ------------------------------------------------------------------------------
participants (8)
-
Asif Lodhi -
Daniel James -
Ferentinos, Jerry -
Kevin Scarr -
Nat Goodspeed -
Robert Dailey -
Scott McMurray -
Steven Watanabe