Hi Peter,

Hm, haven’t thought of that - it makes sense with multi tracker torrents but I think it would be rare that all are queried at the same time (maybe only as the multitracker torrent is initially started). Let’s create a separate request for each announceUrl and move announceUrl to the TrackerRequest.

Ok, I agree.

OK, let’s keep the classes separate! I think that the right place for eventToString() is in TrackerRequestWriter. It knows the binary format of the request and evenToString() returns the binary format of the event field. No user of TorrentRequest would need to know how this field maps to a URI query item value.

The reason why I left only eventAsString() was that adding a new event to the TrackerRequest would cause only this class to be updated, not anything else (I mean TrackerRequestWriter). But to be consistent, I’ll move this function to the TrackerRequestWriter, so TrackerRequest will be just an “advance struct”.

I suggested passing flags to encoded() and unencoded() because your code in getPathForHttpHeader() actually concatenated the path and query components (to produce the URI for the GET request) and thus duplicated the logic already implemented in Uri. I agree that in addition to extending encoded() and decoded() we could add a parameter to each of the setters defaulting to unencoded. Similarly, we could add a parameter to the getters defaulting to unencoded.

Ah, I absolutely forgot that I was using path + query when I was writing these examples. Ok, so maybe we could use both approaches, so a user of this class can choose what’s better for him?

Yet another option would be to provide static helpers that can encode/decode strings (an issue here is that we exclude different chars from the encoding).

I think that the approaches above should be enough. That issue could be solved by adding an extra parameter(s) like excludeChars and includeChars (QUrl::toPercentEncoding() style), however the approaches above seems fine and sufficient to me.

Have a nice day,

Petr