Thanks this is great mail!
Stef
On Oct 9, 2008, at 10:18 PM, nicolas cellier wrote:
>> http://bugs.squeak.org/view.php?id=7180 +6455 +1603 +1602
>>
>> Patches for Interval indexOf: and includes:
>
> I wonder about these fixes because of
http://code.google.com/p/pharo/issues/detail?id=35
>
> Did you reconsider the correctness of these fixes? They seem
> complicated and after reading through the bug reports again
> (especially http://bugs.squeak.org/view.php?id=7180) I concluded
> that they may introduce more problems than they fix.
I know I tried to find my way there too.
Now I trusted nicolas.
Nicolas are you strong on these fixes?
Stef
>
>
> Adrian
> _______________________________________________
> Pharo-project mailing list
> Pharo-project at lists.gforge.inria.fr
> http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
>
Thank you Stef.
Yes, the fix is safe.
I develop here after these 3 points
A)- The code was wrong
B)- Discussions were too long, but the fixes are very easy
C)- library could be enhanced
Adrian, hope I will convince you (also, read the code).
Cheers
Nicolas
A) The code was wrong
Prior this patch:
- #includes: implementation were WRONG.
- #indexOf: was missing, falling back to naive super slow
implementation
Try these two tests in any Squeak image of your choice (3.7 to: 3.10):
"There is no reason that the first is true while the second is false"
{(Float pi to: 4*Float pi by: Float pi) includes: Float pi*2 +
1.0e-11.
(Float pi to: 4*Float pi by: Float pi) includes: Float pi*2 -
1.0e-11}.
"There is no reason for this timeToRun to grow"
(1 to: 7) collect: [:i |
[(1 to: 10000000) indexOf: (10 raisedTo: i)] timeToRun]
B)- the fixes are very easy
I simply implemented indexOf: with same algorithm as #includes:
(corrected!).
Then removed includes: because super includes: is implemented
sending indexOf:
Then I removed valueIncludes: because includes: was sole sender.
You see, two methods removed, one fix added: no revolution
C)- library could be enhanced
What I would prefer is:
- implement a method with an argument so that user can choose
comparison tolerance
- expand this fuzzy inclusion test to any other collection
Maybe something like:
includes: aNumber moreOrLess: aTolerance.
Then Interval includes: could eventually use a default tolerance.
But the obvious bugs must first be corrected...
_______________________________________________
Pharo-project mailing list
[email protected]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
_______________________________________________
Pharo-project mailing list
[email protected]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
