Mantis - Squeak
Viewing Issue Advanced Details
6560 Collections minor always 07-13-07 08:47 02-06-11 23:48
closed 3.9  
none trunk  
0006560: [BUG][FIX] "{Color red. Color green. Color blue} sum" can't return "Color white"
Color red + Color green + Color blue "=> Color white "
{Color red. Color green. Color blue} sum "=> Color cyan "

has duplicate 0007526closed  [BUG][FIX] "{Color red. Color green. Color blue} sum" can't return "Color white" [^] (465 bytes) 07-13-07 08:47 [^] (655 bytes) 07-14-07 04:37 [^] (2,153 bytes) 07-14-07 04:37 [^] (461 bytes) 05-06-10 07:10

07-14-07 04:37   
Hi sumi,

A great bug find.

Looking at your fix, i realized
injecting (sample - sample) gives you the zero element . So I tried that and wrote some tests to prove it still worked.

Uploaded are the fix

and bug tests

the four color sum test fail until the patch is provided.

If you can think of other ways to torture sum, please add to the tests.

07-14-07 11:23   
Thank you for your response.
The injecting (sample - sample) is nice idea!
07-17-07 03:43   
"fix begin"
Installer mantis bug: 6560 fix: ''.
"fix test"
Installer mantis bug: 6560 fix: ''.
"fix end"

08-12-08 07:23   
Sorry, but I don't like the solution here. The case obviously calls for a implementation that doesn't use #inject:into:.

A cleaner solution is to add a #reduce: method to Collection or SequencableCollection and implement #sum in terms of that.

Just to be clear, there is a good implementation of #reduce in the Magritte-model package that looks like so:

reduce: aBlock
    | result |
    result := self first.
    2 to: self size do: [ :index |
        result := aBlock
            value: result
            value: (self at: index) ].
    ^ result

So once we have this method in core, #sum can be simply:

     ^ self reduce: [:accum :each| accum + each ]

Simple, well factored and elegant with no unnecessary work done.
08-12-08 10:33   
I think introducing #reduce: (or a sort of #inject:into: without initial value argument) is very nice idea. But your implementation is not suitable.

Both of your #sum and #reduce: don't work properly as you wish. Please try below.

| set |
set := {Color red. Color green. Color green. Color blue} asSet.
set sum "=> Error "
set reduce: [:sum :each | sum + each] "=> Error "
08-14-08 03:23   
Hi guys,

I note that the discussion on squeak dev took an early academic turn.

When I got to this problem I was just trying to fix a bug and write a test to prove that
a) there was a bug (run test w/o patch)
b) the patch fixed it.

The rest was just.
1) Do the simpliest thing that might work.
2) Respect my time and other projects by leaving it at that,
       Respecting the "your-not-gonna-need-it " principle.

3) There is also an implicity rule to respect the external language of squeak.
That is, if a message is used for a specific behavior. Then the fix needs to preserve that meaning and behavior.

Here it means sum works for all collections so the fix needs to as well.
It has to retain its generality.

4) Elegance and economy are also important.
Intention revealing code. Simplicity.

4a) going easy on the gc.

The reduce code looks sort of cool.
Sumi's points out that it raises errors when used for sum.
This suggest that sum needs an sunit test
 that tests many cases of collection.

This would apply to reduce as well if it also were spec'ed to be general.

So, jbj, believe in your idea enough to make some working code and tests that prove it works?

I would suggest you offer it in its own report as an [enh] .

Yours is service and curiosity, --Jerome Peace

01-09-10 02:49   
"fix begin"
Installer mantis bug: 6560 fix: ''.
"fix test"
Installer mantis bug: 6560 fix: ''.
"fix end"
01-09-10 02:53   
This bug still exist in Squeak3.11-8720-alpha.image!
Which can be seen by loading and running the tests.

Well it has been over a year since I last responded.
My recommendation is to harvest the fix I proposed. It is sound and valid.

Yours in curiosity and service, --Jerome Peace
05-04-10 15:25   
This bug still exist in Squeak4.1
05-04-10 15:57   
Thanks for the reminder. It is now fixed in trunk.
05-06-10 07:22   
I also fix the Bag>>#sum in the same wiz's manner (sample-sample for generate zero element) and attached as

{Color red muchDarker. Color green muchDarker. Color red muchDarker. Color blue muchDarker. Color blue muchDarker. Color green muchDarker} asBag sum "=> Color white is more desirable than raising exception"
05-06-10 11:56   
I notice that Bag>>#sum has the same problem. Please fix it also. Thanks.
11-07-10 01:00   
Bag >> #sum is fixed in Collections-ul.403.