Uri class and parser

[Mike van Dongen]

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/mohceehplxdhsdllxkzt@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 
;)