Hello everybody,
Here is yet another patch related to BitTorrent packets. I’ve implemented BitTorrent’s PacketFactory with extractPacket() and createPacket() methods.
According to the design of PacketFactory, extractPacket() looks into the raw bytes, which are probably read from a Connection, and tries to figure out the header/payload/trailer lengths of the first packet available in the raw buffer. After the packet is extracted, createPacket() can be called with the corresponding data. If it is possible to identify the type of the packet from the raw data, then a default-constructed Packet object of the correct dynamic type is created. After that PacketFactory tries to Packet::parse() the packet and if that fails a packet of type BadPacket is created and returned. A BadPacket object contains just the bad raw bytes, which could be dumped for debugging or diagnostic purposes.
I think I should better now remove all ctors that take the raw data as arguments and call parse(). If we want to force that a packet is always valid, then only PacketFactory should be allowed to call parse(). That can be ensured by making parse() private and declaring PacketFactory as a friend class.
Removing the raw data ctors would require also some changes in the unit tests for the BitTorrent packets. Regarding unit tests, I don’t feel I’m doing the tests really fine right now. For example I always make a test fixture but don’t really declare any shared data for all tests. Some test functions I have written should better become test suites containing the sub-scopes as separate functions. I feel like I need to read more about CppUnit and unit testing in general. I just got Kent Beck’s “Test Driven Development by Example” and I hope to get enough time to read it soon. Could somebody recommend me some other good books on the topic?
Best regards,
Peter
P.S. Alternative to the patch is my BitTorrent branch available at http://bzr.calitko.org/developers/peter/calitko-BitTorrent
