Mike van Dongen wrote: > Hi all! > > I've been working on an URI parser which takes a string and then > separates the parts and puts them in the correct properties. > If a valid URI was provided, the (static) parser will return an > instance of Uri. > > I've commented all relevant lines of code and tested it using > unittests. > > Now what I'm wondering is if it meets the phobos requirements and > standards. > And of course if you think I should do a pull request on GitHub! > > My code can be found here, at the bottom of the already existing > file uri.d: > https://github.com/MikevanDongen/phobos/blob/uri-parser/std/uri.d
Just some small nit-picks. * Rename URIerror to URIException and make it derive from Exception I assume it is allowed to recover from such errors. * URI_Encode => uriEncode in general checkout the Phobos style guide regarding function names etc. http://dlang.org/dstyle.html * 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. * Maybe you should add a constructor in addition to URI.parse() to construct some simple URIs. * Check whether it is const correct. Is const used where it makes sense? * You could implement opEquals to allow checking for equal URIs. * Should URIs be only movable? Then you should add @disable this(this); * The code looks sometimes a bit C-ish (gotos) and deeply nested. Maybe some functions could be refactored to ease maintenance. * Regarding documentation. Please state whether it works in CTFE. Probably add some unittests. I hope the module makes it into Phobos. I suggest std.net.uri. Thank you very much for contributing. Jens