Notes |
(0002955 - 351 - 383 - 383 - 383 - 383 - 383)
sam
10-24-05 16:29
|
The "- 1" in "limit := max asInteger - 1" seems erroneous. Also, by reading and testing the code, I noticed that in every case it returned the primes up to 11, so I added an extra test for this to call #primesUpTo:do: instead of #largePrimesUpTo:do: in this special case.
I'll attach a proposed changeset (no Monticello setup where I am right now). |
|
(0002967 - 272 - 302 - 302 - 302 - 302 - 302)
andreas
10-25-05 09:15
|
Depends on your definition of "up to". The one I used was the one from Stream which implements #upTo: by "answering a subcollection from the current access position to the occurrence (if any, but not inclusive) of anObject in the receiver". Notice the "but not inclusive". |
|
(0002968 - 73 - 98 - 98 - 98 - 98 - 98)
sumi
10-25-05 10:20
|
Integer primesUpTo: 5 " => #(2 3 5) "
seems to include 5, doesn't it? |
|
(0002969 - 52 - 52 - 52 - 52 - 52 - 52)
andreas
10-25-05 11:52
|
Sure does. But does that invalidate my argument? ;-) |
|
(0002970 - 278 - 290 - 290 - 290 - 290 - 290)
sam
10-25-05 11:54
|
Andreas: what is *certain* at this time is that the behaviour is bogus and inconsistent. I would propose to apply my CS first then maybe to change the code so that the meaning of upTo is fixed if needed.
Otherwise, nothing will get fixed and the behaviour will stay as it is. |
|
(0002979 - 465 - 465 - 465 - 465 - 465 - 465)
andreas
10-25-05 21:44
|
All I'm asking for is a justification, any justification for why primesUpTo: should indeed include the argument. You say it's inconsistent, and I agree, but that doesn't necessarily mean it must be changed in the way you propose. I have given an argument for doing it the way I decided to implement the method (consistency with naming conventions in Stream) and if you have an argument for why the method should include the argument (if prime), I'd like to hear it. |
|
(0002982 - 538 - 672 - 672 - 672 - 672 - 672)
sumi
10-26-05 05:49
|
sam,
Thanks your supplementary explanation.
andreas,
As sam said, I think that the problem is the dependence of behavior of #primesUpTo: upon its argument (larger than 25000 or not). Definition of "up to" is the same of yours, not inclusive of arg (even if it is prime, in this case). I apologize that the title of this report is not proper. The bug is in #primesUpTo:do:, but not in #largePrimesUpTo:do:.
So, the Integer class >> #primesUpTo:do: of
1 to: limit do: [:i | ...
should be
1 to: limit - 1 do: [:i | ... |
|
(0003716 - 544 - 1004 - 1004 - 1004 - 1004 - 1004)
MarcusDenker
02-08-06 20:56
|
I added tests
testPrimesUpTo
"upTo: semantics means 'non-inclusive'"
primes := Integer primesUpTo: 5.
self assert: primes = #(2 3).
"this test is green for nn>25000, see #testLargePrimesUpTo"
nn := 5.
self deny: (Integer primesUpTo: nn) last = nn.
self assert: (Integer primesUpTo: nn + 1) last = nn.
and
testLargePrimesUpTo
| nn |
nn := (2 raisedTo: 17) - 1.
self deny: (Integer primesUpTo: nn) last = nn.
self assert: (Integer primesUpTo: nn + 1) last = nn.
with testPrimesUpTo failing in 3.9a |
|
(0003777 - 5 - 5 - 5 - 5 - 5 - 5)
MarcusDenker
02-13-06 14:41
|
fixed |
|