Mantis - Squeak
Viewing Issue Advanced Details
6560 Collections minor always 07-13-07 08:47 02-06-11 23:48
sumi  
andreas  
normal  
closed 3.9  
fixed  
none    
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" 
 Collection-sum.st.gz [^] (465 bytes) 07-13-07 08:47
 Collection-sum-wiz.st [^] (655 bytes) 07-14-07 04:37
 SumBugs.st [^] (2,153 bytes) 07-14-07 04:37
 Bag-sum-sumi-M6560.st [^] (461 bytes) 05-06-10 07:10

Notes
(0010875)
wiz   
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
Collection-sum-wiz.st

and bug tests
SumBugs.st

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.

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

(0012484)
jbj   
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:

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


Simple, well factored and elegant with no unnecessary work done.
(0012485)
sumi   
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 "
(0012502)
wiz   
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

(0013458)
wiz   
01-09-10 02:49   
"fix begin"
Installer mantis bug: 6560 fix: 'Collection-sum-wiz.st'.
"fix test"
Installer mantis bug: 6560 fix: 'SumBugs.st'.
"fix end"
(0013459)
wiz   
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
(0013749)
sumi   
05-04-10 15:25   
This bug still exist in Squeak4.1
(0013750)
andreas   
05-04-10 15:57   
Thanks for the reminder. It is now fixed in trunk.
(0013753)
sumi   
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 Bag-sum-sumi-M6560.st.

"example"
{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"
(0013758)
sumi   
05-06-10 11:56   
I notice that Bag>>#sum has the same problem. Please fix it also. Thanks.
(0013905)
leves   
11-07-10 01:00   
Bag >> #sum is fixed in Collections-ul.403.