My first D module - Critiques welcome.

[bearophile]

John Carter:

> I'm storing the roman numerals as dchars. I have the vague hope 
> this is more efficient than storing them as strings.

From/to Roman numbers conversions are never performance-critical. 
It looks like premature optimization.


> What do you mean by "missing UCSF"?

Sorry, it's UFCS (Universal Function Call Syntax), it means 
writing x.foo instead of foo(x).


> I would love away around that.

http://dlang.org/phobos/std_exception.html


>> That's quite bad code. Better:
>>
>>     return roman.map!romanToDigit.array;
>>
>>
> That certainly looks nicer, alas, dmd is unhappy....

When you have only one argument the () is not needed:

return roman.map!(a => romanToDigit(a)).array;

The cause of your error: romanToDigit is module-private. If you 
make romanToDigit public the error goes away. If you use a lambda 
the problem also goes away for different reasons. So in this case 
using a lambda is OK.


> I thought it meant just putting a ddoc /// extra / flag on the 
> comment,

Right. The Ddoc-produced html should show the unittest.

You can also add a module-level ddoc to your module.


return find!((a,b) =>

In most cases it's better to put a space after every comma ==>

return find!((a, b) =>


> };

In D don't put a ; there for struct/classes/unions.

I think the code density of the toRomansWithZero() function is 
too much low. I suggest to pack it vertically a little more. D is 
not C#.


O suggest you to be more careful with size_t, your code doesn't 
compile on a 32 bit system:

test.d(59): Error: cannot implicitly convert expression 
(this.regionIndex) of type const(ulong) to uint
test.d(62): Error: cannot implicitly convert expression 
(this.regionIndex) of type const(ulong) to uint
test.d(62): Error: cannot implicitly convert expression 
(this.regionIndex) of type const(ulong) to uint
test.d(66): Error: cannot implicitly convert expression 
(this.previous) of type const(ulong) to uint


So this could be better:

    size_t regionIndex; /// Some digits in the system "dominate" 
over others.
    size_t previous;    /// Hardcode link to previous digit.


I also suggest to add an ending "." to comments.

With this I think the surface review of your code is mostly done.

Bye,
bearophile