On 15.07.2011 02:20, dsimcha wrote:
Lars Kyllingstad's new and improved std.path module for Phobos is the
next item up in the review queue.  I've volunteered to be the review
manager.  Barring any objections, the review starts now and ends at the
ends two weeks from now on July 28.  This will be followed by a week of
voting, ending August 4th.

Code:
https://github.com/kyllingstad/phobos/blob/std-path/std/path.d

Docs:
http://www.kyllingen.net/code/new-std-path/phobos-prerelease/std_path.html


Looks nice and clean, both docs and code! I like modules that are not overengineered :)

I noticed a couple of things that I don't think have been mentioned already:

The docs for defaultExtension say that there's one case where doesn't allocate. But the code says that it always does (.idup isn't conditional, it's just .dup with a different return type).

joinPath also uses .idup, thus always allocates. Maybe there could be a documented allocation policy for the module as a whole? Copy-on-write used to be the general rule for Phobos, don't know if that's true anymore. For most of the functions in this module, even a single heap allocation could be more expensive than the rest of what they do.

I've read the discussions about the function names, but I still dare to make a suggestion of my own. I feel like joinPath is a bit iffy, shouldn't it be plural somehow? What it does is to create a path by putting pieces together, some paths in their own right, some not. My suggestion is to simply call it buildPath.

Reply via email to