Hi Petr,

I’ll try to sumarize it, so we’ll see what needs (should) to be changed. I’ve checked the {Torrent, TrackerResponse}Parser classes and I think that there is currently no problem with Torrent metainfo parsing and validation, so we don’t need an extra validation class for it and we should focus only on the TrackerResponseParser class - do you agree?

Yes, I agree, let’s concentrate on TrackerResponseParser - if the resulting design turns out to be significantly better we might consider refactoring TorrenParser too.

Your code snippet seems fine to me. So, if I understand it correctly, let me describe the ‘peers’ parsing process one again, taken your code snippet in mind:

* ‘ip’ - just get a raw string and do nothing special with it
* ‘port’ - get a number, check the length and if it’s greater than quint16::max() or negative, then set it to 0, otherwise cast it to quint16 -
* ‘peer_id’ - get a raw string, check it’s length and if it’s not valid, then pass an empty string instead of it

I thought about PeerId being a VendorId like class (possibly a typedef for FixedSizeArray), so PeerId() will create a PeerId object internally storing a 20 bytes zero filled QByteArray.

“Peer id can be either a string that’s 20B long (that means that it was parsed correctly), or an empty string (original peer id was incorrect).”

We would not need stuff like that explicitly stated in the docs if we use a type that enforces certain invariants - PeerInfo is a FixedSizeArray <20> which is always 20B long and the default value is all null 20 bytes.

Let me know what you think about what I’ve written. Maybe I can then refactor Tracker{Request, Response} to use Uri instead of the current PeerInfo, remove the PeerInfo class (I think we won’t needed it, will we?), update TrackerResponseParser (’peers’ parsing and loading) and other related classes (like TrackerRequestSession).

Yes, I think Uri could replace PeerInfo. If might be a good idea though define typedef Uri PeerInfo in module PeerInfo. I think it’s a good idea to use “concrete” types (PeerInfo) rather then generic ones (QByteArray) even if that would mean using a simple typedef. This approach would allow to easily change the mapping to a generic type or even implement a custom one. I’ve not used this approach very often, but that seems like a good idea to me! What do you think?

Regards,

Peter