Wow mate I had a quick look and that looks pretty neat…
I just have one suggestion however… How about using NULL OBJECT so that all the
if( !isValid() )
checks need not be done…
The idea is there is only one invalid packet object ( a singleton ) that returns an empty QByteArray() when asked to execute rawheader() etc… )
As the NULL OBJECT obeys the same interface as packet it will always be correct ( i.e. there is no need for the checks and the checks are never missed )
I will submit a patch after a week but this is on my mind for long so what do you all think?
See
http://www.refactoring.com/catalog/introduceNullObject.html
for example…..
— cheers atul
Peter Dimov wrote:
Hello everybody,
I finally publish some preliminary version of the Packet base class, which should be used as a base for Gnutella, BitTorrent and other future protocol packet implementations. It should make it easier to add support for a new protocol based on exchange of binary messages. In this post I have presented some of the questions I’ve been asking myself regarding the design of Protocols::Generics::Packet. I hope that I will get some feedback from at least some of you. Exchanging some ideas would definitely contribute to achieving a better solution.
I based my current work on the precious Gnutella Packet implementation. Some documentation may thus not be adequate. The function comments will be moved to the cpp file, so that lines not exceed a length of 80 characters.
I’m wondering whether the trailer functions should be commented out or removed until we really have to implement a protocol that uses some trailer data. Otherwise I would be adding something that never gets used, or is inadequate to a later arising need.
Just as an idea, we might at a later point add some functionality to sign packets.
Are the pure virtual functions name() and protocol() required? They might be of help for debugging purposes or for classes like PacketDump or PacketMonitor. I’ve added an accept() function for the visitor patter which Atul applied on the Gnutella Packet class. When we have the visitor pattern we might create a concrete visitor that returns the name or protocol of a packet object.
I also have the idea to declare structs for Header, Payload and Trailer, from which the derived Packet classes should derive their own structs. The point is to have the Packet base class to take care of reference counting (there are some examples for that in the source of Qt). What do you think of that?
It would maybe also make sense to derive the GGEP extensions base class from the generic Packet.
I still haven’t figured out how to ensure some invariants like length of raw payload data be the same as the value in the packet header. Maybe the derived classes should care about this. I guess I’ll have to do first experiments with BitTorrent and get some feedback.
Regards,
Peter
