On Thursday, 25 October 2012 at 13:59:59 UTC, Jens Mueller wrote:
Just some small nit-picks.
That's what I was hoping for :D
in general checkout the Phobos style guide regarding function
names
etc.
http://dlang.org/dstyle.html
add some unittests.
Thanks! I haven't read that page before, but I think my code is
already in that style.
That page referred to one about code coverage, which I didn't
know is an option in dmd.
I added some unittests and increased the code coverage to 100%.
http://dlang.org/code_coverage.html
* Rename URIerror to URIException
and make it derive from Exception
I assume it is allowed to recover from such errors.
* URI_Encode => uriEncode
I know it was hard to tell, but that's not my code.
* The code looks sometimes a bit C-ish (gotos) and deeply
nested. Maybe
some functions could be refactored to ease maintenance.
I'm not sure what functions you mean.
I hope the module makes it into Phobos. I suggest std.net.uri.
I agree I should move it, so moved my code into a new file,
'std/net/uri.d'.
https://github.com/MikevanDongen/phobos/blob/uri-parser/std/net/uri.d
* Why is Uri a class and not a struct? Do you expect people to
derive
from Uri? But then you need to check whether URI.init makes
sense.
According to the Phobos style Uri should be renamed to URI.
The renaming is done. At the start, the class was quite general
in a way I wanted to make others derive from it. That plus the
little experience I have with structs in D made me use a class.
Now I don't see a reason why anyone would derive from it, so I
think it should either be a final class (as Jacob Carlborg
suggested) or a struct.
* Maybe you should add a constructor in addition to URI.parse()
to construct some simple URIs.
Do you mean with parameters (strings?) that simply set the
properties?
* Check whether it is const correct. Is const used where it
makes sense?
Been thinking about that myself aswell. Both for return values
and parameters.
* You could implement opEquals to allow checking for equal URIs.
I like this suggestion! :)
* Should URIs be only movable? Then you should add
@disable this(this);
http://forum.dlang.org/thread/mohceehplxdhsdllx...@forum.dlang.org
I'm not sure what movable means or what @disable does. That
thread doesn't explain it means very clear.
* Regarding documentation. Please state whether it works in
CTFE. Probably
I'll look that up ;)
I hope the module makes it into Phobos. I suggest std.net.uri.
Thank you very much for contributing.
Jens
Again, thanks for being supportive; that's what makes me continue
;)
