std.rational?

[H. S. Teoh]

On Fri, Sep 27, 2013 at 03:47:20PM +0200, Joseph Rushton Wakeling wrote:
[...]
> I've made a fork <https://github.com/WebDrake/Rational> and tweaked
> the code to be in line with current D/Phobos style (David Simcha's
> last update is from over 2 years ago so it's out of date in that
> respect).  I did this manually, line by line, so I could have a
> proper read-through of the code.
> 
> On the basis of that read-through it looks pretty comprehensive and
> well-thought-out.  There are various workarounds for bugs which
> probably need to be re-examined depending on whether those bugs have
> been fixed or not.  In any case it compiles fine with current DMD,
> and the unittests are thorough and have 94% code coverage -- good
> job, David!
> 
> One thing I don't like: David uses "num" and "denom" for the public
> methods to get numerator and denominator.

For the record, I'm OK with it. I don't like "numerator" and
"denominator" as it's needlessly verbose. It's already clear what "num"
and "denom" mean in a rational number library, no need to spell it all
out.

But, others may have a different opinion on this. As far as the review
is concerned, I don't mind either way.


[...]
> Anyway, if anyone else wants to take a look through and give me any
> comments, they'd be welcome.

I'll see if I can take a look at it sometime -- no guarantees, though,
as I'm busy with other things these days.


> (Bearophile, I will add your requested Fraction, but I'd like to get
> more info on what everyone else thinks of the code as it is before
> making any additions:-)
[...]

Honestly, I don't think that's necessary. It's just a one-line alias,
trivial to add in user code. For Phobos code, I think writing
Rational!BigInt is better, to make it clear what underlying number type
is being used to instantiate the template.


T

-- 
One Word to write them all, One Access to find them, One Excel to count them all, And thus to Windows bind them. -- Mike Champion