Uri class and parser
Jens Mueller
jens.k.mueller at gmx.de
Thu Oct 25 14:34:33 PDT 2012
Mike van Dongen wrote:
> 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
Are you sure? I've seen _ in names where camel case should have been used.
> >* 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.
You can still change it, can't you?
Just want to see the code being improved.
> >* 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.
There are functions that I find difficult to understand. Usually the
ones with lots of deeply nested control statements. And often they are a
bit long. I will read it later again and be more specific.
> >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
Now we need to properly deprecate the old std.uri without breaking any
code. Do you think this is feasible?
And are the Phobos core developers okay with this?
> >* 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.
Do you want reference semantics or value semantics? I find value
semantics easier. So I would go with a struct. When passing URIs around
one can still pass by reference.
> >* 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?
Yes.
At least to support constructing often used URIs.
And then maybe another constructor that calls parse for all other cases.
> >* 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.
It's annoying to do it later but it should be done, I think. And it's
not too late. Good luck!
> >* 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.
In my understanding structs are moved by default. That means the members
a bit-wise copied. Now assuming you move a pointer, then you may want to
copy the contents that the pointer to have an independent object, a copy
so to say.
This is achieved by using the so called postblit constructor this(this).
If you disable it, then the URIs are only movable. If you want copying,
then you may need to implement it.
http://dlang.org/struct.html#StructPostblit
> >* 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 ;)
I hope there will be a happy end.
We should probably seek some advice from the core Phobos developers
whether they want to have an improved std.uri moved to std.net.uri.
Don't want you to put work into something that is unlikely to make
through the review process.
Jens
More information about the Digitalmars-d
mailing list