<br><br><div class="gmail_quote">On Sat, Aug 28, 2010 at 02:48, David Simcha <span dir="ltr"><<a href="mailto:dsimcha@gmail.com">dsimcha@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin: 0pt 0pt 0pt 0.8ex; border-left: 1px solid rgb(204, 204, 204); padding-left: 1ex;">
I've cleaned up/fixed the bit rot in my Rational library. It's available for review at: <a href="http://dsource.org/projects/scrapple/browser/trunk/rational/rational.d?rev=785" target="_blank">http://dsource.org/projects/scrapple/browser/trunk/rational/rational.d?rev=785</a> .<br>
</blockquote><div><br>Let's give it a try.<br>First thing, it's not your faut, but:<br><br>import rational;<br>void main()<br>{<br> auto r = rational(1,2); // error, finds module rational and not rational.rational<br>
}<br><br>This is going to be a PITA...<br><br>Let say from nom on, I prefix everything with rational.*<br><br>auto r = rational(1,2);<br>r = 1; // do not work.<br><br>Maybe you could add opAssign from integer numbers?<br>
<br>Next, 1/0. OK, it's correctly dealt with (Integer Divide By Zero Error)<br>But 0/1 botches. In fact, any rational that's zero creates a DBZ error. r-= r, etc.<br>I guess it's a pb in simplify (454-455) or gcf (638), you should test for a zero numerator or divisor<br>
<br>Now, for the code itself:<br>line 35: Maybe you should add %, %=, they are part of what I consider 'integer like'. You use them in gcf anyway<br><br>As an aside, isIntegerLike, the equivalent isFloatingPointLike and defineOperators!( some string tuple) could be interesting additions to std.traits. <br>
I don't know what FP-like would be, maybe test for values between 0 and 1, and test for NaN.<br><br>75: if you make it public, you should add a template constraint if(isIntegral!I1 && isIntegral!i2). The same for gcf at line 640<br>
76: why * instead of +?<br><br>303: swap in invert. I like this!<br><br>307, 317 (opEqual, opCmp). Wikipedia uses denom1*num2 - denom2*num1 == 0 as the equivalence relation among rational numbers. Maybe this can be extended to opCmp ? Ah, you want to avoid overflow, I gather? OK, scratch this.<br>
<br>559: could toRational be a constructor? auto r = rational(PI, 1e-8);<br><br>638, 675: Maybe gcf/lcm could become part of std.numerics?<br><br>All in all, I like you code very much, it's very clean and readable. It's even organized the way I'd do it : first templates, then structs, then functions.<br>
<br><br>Some possible additions: <br>* an integerPart function or property (returning an Int) or casting as an Int directly, 22/7 -> 3<br>* a fractional part function (ie 22/7 is ~3.14 -> 0.14) as a FP<br>* Accordingly, floor/ceil functions could be interesting, unless you think the user should use std.math.floor(cast(real)rat), which I could understand.<br>
* Addendum (probably not a good idea): you can have infinity and NaN-like
values with 1/0, -1/0 and 0/0...<br><br>***<br> 2.
I couldn't figure out how to handle the case of user-defined
fixed-width integer types in the decimal conversion (opCast) function,
so it just assumes non-builtin integer types are arbitrary precision.
The biggest problem is lack of a way of introspecting whether the
integer is fixed or arbitrary width and what its fixed width might be if
it is, in fact, fixed. The secondary problem is that I don't know of a
good algorithm (though I'm sure I could find one if I tried) for
converting fixed-width integers to floating point numbers in software.
Arbitrary precision is much easier.<br>
***<br><br>About converting fixed-width integer-like types, maybe these should provide .min and .max properties?<br>That way you know the range of possible values and maybe it's then possible to convert (I didn't think this through so I may be completly mistaken)<br>
<br>Good work David. My only pb is that you write code faster that I can read it, and don't let me any time to write some myself! It took me a week to read the recent additions in Phobos!<br><br><br>*goes back to cleaning his algebraic data types and type mimicry code*<br>
<br><br>Philippe<br><br>Philippe<br></div></div>