Hi Nocolas,

Thanks for the review.  I have not actually used this for arithmetic yet, I
added that in a couple of hours before publishing this.  Previously this has
been used to represent the decimal values that I needed to use to load into
a database using a fixed length representation.  This need influenced my
design quite a bit.  However, without arithmetic, it isn't very useful for
many other people (or myself going forward).

As to your points;


(-53/10 asFixedDecimal: 2) fractionPart.


Right.  I  did not finish adding in all of the protocol that should be
added.  I will  continue to do this over the next few days as I have time.
For now, this works:

(-53/10 asFixedDecimal: 2) fractionPart. -0.30

- why maintaining so many forms in parallel ?
>    Not bug-proof, you happen to update one form not the other...
>    sign is not handled in part1 nor part2


This is a result of  what I was originally using it for, which works pretty
well (until I introduce bugs, that is).  Keeping the various different forms
there does make certain transformations much easier to handle.  However, I
might take a look at other ways to handle this.  (Also, this repository is
open, is if anyone wants to add a different representation, they are free to
do so).

- negated should create a new instance
>    rather than modifying the number in place


Yes, that was a bad choice.  It is now making a new instance.

- FixedDecimals does not implement hash,
>    therefore cannot be stored in a Dictionary.

   Particularly, cannot be stored in a workspace variable.
>    I suggest hash ^self asFraction hash


This is now fixed, as you suggested.  Although I may tweak this (is there a
good hash tester available for Squeak?)

- likewise, implement all subclassResponsibility


Right.  I'm working on that...

        ( 5/3) * (5/3 asFixedDecimal: 2).
>         ( 2.0s1) * (5/3 asFixedDecimal: 2).
>         (5/3 asFixedDecimal: 2) reciprocal.


Fixed.  See below.

- coercion depends on receiver:
>         | a b |
>         a := (53/10 asFixedDecimal: 2).
>         b := 5.3.
>         { (a+b) class.
>         (b+a) class. } "is {FixedDecimal . Float}"


Hmmm.  Yes.  I think I want all arithmetic with FixedDecimals to return a
FixedDecimal.  Looking as ScaledDecimals, I notice that what is returned is
consistent with for a given other magnitude, but not always the same.  Is
there a rational behind which type of number is returned?  Is it spelled out
in ANSI?
(Also, while checking this, I noticed that .333 does not give you 0.333 but
rather 333.  I wonder if I've coded in a bug like that somewhere else?  If
the . is the first part of the method or workspace, it just quitely eats
it.)

- maybe add another rounding mode in arithmetic (to nearest even...)


I might add something like that as time permits.  Do you have a use for it?

- create a specific notation for transparent read/write, like 5.30f
>   (requires some easy support in Number class>>readFrom: or NumberParser)


Will do.

>From algebraic point of view, the main concern is that some numbers
> might not have any reciprocal...
> Unless the reciprocal is a Fraction, and arithmetic op coerce to
> Fraction...
> But, eh, this is just the case of Float:
> | f |
> f := (8872240531573379/9007199254740992) asFloat.
> f * f reciprocal - 1


Well, this particular number DOES have a reciprocal in FixedDecimals which
just happens to be a FixedDecimal.  You just have to specify the scale
right:
fd _ (8872240531573379/9007199254740992) asFixedDecimal: 53
rfd _ fd reciprocal
rfd reciprocal
fd * rfd - 1 = 0.00000000000000000000000000000000000000000000000000000
That said, there might be some rational numbers that will not have a
reciprocal, but I can't find them.  For instance:
fd _ (1/3) asFixedDecimal: 5  "= 0.33333"
rfd _ fd reciprocal  "= 3.00003"
fd * rfd - 1  "0.00000"

Most of this is now out in SqueakSource.  I'll do some more as time
permits.  (Also, all of these changes now have tests, too).

Thanks,
Chris
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.squeakfoundation.org/pipermail/squeak-dev/attachments/20070906/edb270ab/attachment.htm