Hi Atul, good points, as always!

My initial intent was to make createPacket() a static function but then I thought it would be nice to define an abstract PacketFactory in Protocols::Generics and have our new Protocols::Generics::PacketSession class be initialized with a concrete PacketFactory object for Gnutella packets or BitTorrent packets or whatever kind of packets we will be exchanging with the remote peer.

Regarding friends, that’s the first idea that came to my mind about how to force a check of the value returned by the new parse() and create a BadPacket object when parse fails. This way only the factory will be allowed to call parse() and we would never get packets in an invalid state (i.e. incompletely parsed). Do you have other ideas? Also having the abstract factory calling parse() saves us from writing the raw data ctors.

Regards,

Peter

atul wrote:

Hi Peter

I am wondering if we need a separate class PacketFactory?

Another design could be to make createPacket a static function of Packet class and use it ( this would make the static method depend on all concrete classes but anyway that is the case with PacketFactory… What do you think?

I am a little hesitant about friends ;-) I mean class friends ;-)
– cheers atul