Mantis Bugtracker
  

Viewing Issue Simple Details Jump to Notes ] View Advanced ] Issue History ] Print ]
ID Category Severity Reproducibility Date Submitted Last Update
0006560 [Squeak] Collections minor always 07-13-07 08:47 02-06-11 23:48
Reporter sumi View Status public  
Assigned To andreas
Priority normal Resolution fixed  
Status closed   Product Version 3.9
Summary 0006560: [BUG][FIX] "{Color red. Color green. Color blue} sum" can't return "Color white"
Description Color red + Color green + Color blue "=> Color white "
{Color red. Color green. Color blue} sum "=> Color cyan "

Additional Information
Attached Files  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

- Relationships
has duplicate 0007526closed  [BUG][FIX] "{Color red. Color green. Color blue} sum" can't return "Color white" 

- Notes
(0010875 - 405 - 507 - 507 - 507 - 507 - 507)
wiz
07-14-07 04:37
edited on: 07-14-07 04:40

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 - 75 - 81 - 81 - 81 - 81 - 81)
sumi
07-14-07 11:23

Thank you for your response.
The injecting (sample - sample) is nice idea!
 
(0010885 - 141 - 207 - 207 - 207 - 207 - 207)
wiz
07-17-07 03:43
edited on: 07-17-07 03:44

"fix begin"
Installer mantis bug: 6560 fix: 'Collection-sum-wiz.st'.
"fix test"
Installer mantis bug: 6560 fix: 'SumBugs.st'.
"fix end"

 
(0012484 - 716 - 1143 - 1143 - 1143 - 1143 - 1143)
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 - 386 - 454 - 454 - 454 - 454 - 454)
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 - 1322 - 1600 - 1600 - 1600 - 1600 - 1600)
wiz
08-14-08 03:23
edited on: 08-14-08 03:28

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 - 139 - 193 - 193 - 193 - 193 - 193)
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 - 292 - 339 - 339 - 339 - 339 - 339)
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 - 33 - 33 - 33 - 33 - 33 - 33)
sumi
05-04-10 15:25

This bug still exist in Squeak4.1
 
(0013750 - 50 - 50 - 50 - 50 - 50 - 50)
andreas
05-04-10 15:57

Thanks for the reminder. It is now fixed in trunk.
 
(0013753 - 353 - 400 - 400 - 400 - 400 - 400)
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 - 73 - 79 - 79 - 79 - 79 - 79)
sumi
05-06-10 11:56

I notice that Bag>>#sum has the same problem. Please fix it also. Thanks.
 
(0013905 - 43 - 49 - 49 - 49 - 49 - 49)
leves
11-07-10 01:00

Bag >> #sum is fixed in Collections-ul.403.
 

- Issue History
Date Modified Username Field Change
07-13-07 08:47 sumi New Issue
07-13-07 08:47 sumi File Added: Collection-sum.st.gz
07-13-07 09:11 sumi Issue Monitored: sumi
07-14-07 04:37 wiz Note Added: 0010875
07-14-07 04:37 wiz File Added: Collection-sum-wiz.st
07-14-07 04:37 wiz File Added: SumBugs.st
07-14-07 04:40 wiz Note Edited: 0010875
07-14-07 11:23 sumi Note Added: 0010878
07-17-07 03:43 wiz Note Added: 0010885
07-17-07 03:43 wiz Note Edited: 0010885
07-17-07 03:44 wiz Note Edited: 0010885
08-12-08 07:23 jbj Note Added: 0012484
08-12-08 10:33 sumi Note Added: 0012485
08-14-08 03:23 wiz Note Added: 0012502
08-14-08 03:28 wiz Note Edited: 0012502
01-10-09 02:00 Keith_Hodges Status new => pending
01-10-09 02:27 Keith_Hodges Status pending => testing
01-10-09 03:39 Keith_Hodges Status testing => resolved
01-10-09 03:39 Keith_Hodges Fixed in Version  => 3.11
01-10-09 03:39 Keith_Hodges Resolution open => fixed
01-10-09 03:39 Keith_Hodges Assigned To  => Keith_Hodges
01-10-09 03:41 Keith_Hodges Status resolved => testing
10-03-09 19:34 Keith_Hodges Status testing => assigned
10-03-09 19:34 Keith_Hodges Assigned To Keith_Hodges => andreas
01-09-10 02:49 wiz Note Added: 0013458
01-09-10 02:53 wiz Note Added: 0013459
05-04-10 15:25 sumi Note Added: 0013749
05-04-10 15:57 andreas Status assigned => resolved
05-04-10 15:57 andreas Fixed in Version 3.11 => trunk
05-04-10 15:57 andreas Note Added: 0013750
05-06-10 07:10 sumi File Added: Bag-sum-sumi-M6560.st
05-06-10 07:22 sumi Note Added: 0013753
05-06-10 08:35 FrankShearar Relationship added has duplicate 0007526
05-06-10 11:56 sumi Status resolved => feedback
05-06-10 11:56 sumi Resolution fixed => reopened
05-06-10 11:56 sumi Note Added: 0013758
11-07-10 01:00 leves Status feedback => resolved
11-07-10 01:00 leves Resolution reopened => fixed
11-07-10 01:00 leves Note Added: 0013905
02-06-11 23:48 leves Status resolved => closed


Mantis 1.0.8[^]
Copyright © 2000 - 2007 Mantis Group
128 total queries executed.
62 unique queries executed.
Powered by Mantis Bugtracker