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

From: Steven Watanabe (watanabesj_at_[hidden])
Date: 2008-03-29 12:10:01

AMDG

Asif Lodhi wrote:
> Hi Robert,
>
> On 3/29/08, Robert Dailey <rcdailey_at_[hidden]> wrote:
>   
>> On Fri, Mar 28, 2008 at 4:10 PM, Asif Lodhi <asif.lodhi_at_[hidden]> 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