[Cuis-dev] Problems in class Number

[Agustín Sansone]

Hello!

While testing and reading the implementation of the class Number I found
some things that might be cuestionable, some errors and a method that could
have been implemented much faster:

   - In Integer, the *sumTo: anInteger *method,
   computes "self+(self+1)+...+anInteger". The problem here is that if self >
   anInteger this operation does not have sense. For this case, the
   implementation returns 0 instead of rasing an error. This kind of makes
   sense, because not adding anything could be thought as 0, nevertheless this
   is cuestionable.
   - Analogously, the *productTo: anInteger *method, computes
   self*(self+1)*...*anInteger. Similarly, in this case when self > anInteger,
   it returns 1 instead of an error. In this case, I don't think there is a
   direct correlation to think that not multiplying any number should be equal
   to 1.
   - A detail, if now we make this change in the method *productTo:
   anInteger*, there will be a small problem in the method *factorial*.
   This is because what factorial does is *"^1 productTo: self"* (after
   checking if self is not negative). So, with this new change, if we make *0
   factorial* it will raise an error, because we would be computing *"^1
   productTo: 0"*, and this would be not defined. Anyway, this can be
   easily fixed by checking this border case.
   - The *raisedToInteger: exp modulo: m *method in Integer has a very big
   problem. If we compute, for example, *"5 raisedTo: 0 modulo: 0"*, this
   returns 1. This means, that according to Smalltalk, the rest of the
   division by 0 of 1(=5^0) is equal to 1 (Yes, division by zero!!). I think
   you can see the problem. This is due the first line of the method, that
   says *"(exp = 0) ifTrue: [^ 1].", *does not check anything else. This
   problem can be easily fixed by checking if m=0 just before.
   - Now, if we compute *"1 mod: 0" *or (worse) *"(5 raisedToInteger: 0)
   mod: 0"* it raises an error informing that the division by zero is not
   defined (exactly what we could expect if we me made *"5 raisedTo: 0
   modulo: 0"*). So, not only we found an error, but also we found a
   problem of inconsistency in Smalltalk, since for the same calculation we
   can obtain two differents answers.
   - Another detail, in Number there are defined this methods: *mod:
   divisor*, *rem: divisor* y *\\ divisor*. All of this are thought like
   some kind of rest in the division. Particularly, the method *mod:
   divisor *maps to the mathematical operation of modulo of modular
   arithmetic. Smalltalk defines this operation in general for every number as
   modulo, but mathematically this operation should be only be defined for the
   natual numbers, thus it could be added some check for this. "For a
   positive integer n, two numbers a and b are said to be congruent modulo n
   ...[]" <https://en.wikipedia.org/wiki/Modular_arithmetic>
   - Last detail, if we compute *"0 raisedToInteger: 0"* this returns 1. I
   am not a math expert, but I don't think this operation should be defined if
   there is not even a consensual answer for this.
   - The *isPrime *method in Integer makes some optimization in order to
   run the algorithm in O(sqrt(self)) instead of the naive way in O(self).
   This is very intelligent, but the constant factor of this method can be
   still improved significantly. I share with you my implementation of
*isPrimeFast
   *with a small explanation. This implementation runs in general more than
   3 times faster than the actual one. I leave you a test that checks the
   correctness of it as well, and some other tests that check this complexity
   I mentioned.

Please let me know what you think.
Best regards,
Agustín Sansone
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.cuis.st/mailman/archives/cuis-dev/attachments/20191007/b2c0648d/attachment.htm>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: Integer-isPrimeFast.st
Type: application/octet-stream
Size: 819 bytes
Desc: not available
URL: <http://lists.cuis.st/mailman/archives/cuis-dev/attachments/20191007/b2c0648d/attachment.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: IsPrime-Tests.st
Type: application/octet-stream
Size: 1573 bytes
Desc: not available
URL: <http://lists.cuis.st/mailman/archives/cuis-dev/attachments/20191007/b2c0648d/attachment-0001.obj>