Hi Petr,

Thanks for the review! Comments are inline:

As for these UTF-8 conversions - I’d personally first make our Uri class using only ASCII characters (because this is what we currently need). I haven’t studied appropriate RFCs very deeply, though.

I haven’t thoroughly read the RFCs either. I just searched them for UTF-8. But I agree we can do with ACSII for now and leave a \todo about checking the UTF-8 issue for RFC conformance.

What about this way: If invalid data are passed “as one piece” then the whole Uri object will be “set to Null”. If invalid data are passed only to some setter function then simply this setter function will do nothing.

OK, makes sense!

I’d maybe change the names into setAsUnencoded and setAsEncoded because I first thought that these two functions set some kind of a state (encoded/unencoded), but that’s only my impression :).

Agreed!

I’d change the implementation of this function to this one:

if (ch >= ‘a’ && ch = ‘A’ && ch = ‘0′ && ch

or is there some reason to use int literals instead of char literals?

The RFC specified the int literals for ALPHA, DIGIT, etc. I agree it is more readable to use that char literals and I’ll change that!

\todo Should we better use the following regular expression from RFC 3986 to parse the URI?

I’d give it a try (QRegExp).

Me too. I saw the regular expression after I already implemented the function :-(. Could have probably thought about using a regexp myself though.

Maybe we should add there some input validation (e.g. if bytes/key/value is valid/not empty), but that’s related to the validation “question”.

Calling appendQuery() with an empty QByteArray would not have any effect. Calling appendQueryItem() with an empty value should print only the key without a “=” (have to fix that). If only the key is empty, that makes no sense (assert or produce no effect?). If both are empty the function would have no effect.

Change it to UTILS__URI_H to keep consistent naming.

You’re right!

void ensureNotNull (QByteArray &bytes);

I’d made this function either const or static.

Agreed!

// \todo move the typedefs to Uri?

typedef QPair QueryItem;
typedef QList QueryItemList;

Agreed.

Then I’ll move them :-).

Maybe, but I think that we won’t need them in the near future (I won’t need them in the TrackerRequestWriter).

Then I’ll just delete the comment in the test module and move it to the header.

Thanks again for the review!

Good night!

Peter