Re: [Boost-users] Need boost's help with poor design

From: Robert Dailey (rcdailey_at_[hidden])
Date: 2008-03-25 18:24:17

On Tue, Mar 25, 2008 at 4:41 PM, Steven Watanabe <watanabesj_at_[hidden]>
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.