Hi, > On Dec 20, 2015, at 2:30 PM, Max Leske <maxle...@gmail.com> wrote: > > >> On 20 Dec 2015, at 14:10, Tudor Girba <tu...@tudorgirba.com> wrote: >> >> Hi, >> >> Could we not have sum, but sumNumbers instead? So, we would end up with: >> >> sum:ifEmpty: >> sum: (with error) >> sumNumbers (without error) >> >> From the outside, #sum: looks like it should parameterize #sum, but the >> implementation is actually different. So, given that in this implementation >> #sum is not a special case of #sum: the two should be named differently to >> reflect that difference. Hence my proposal is to keep #sumNumbers instead of >> #sum. > > I could live with that. We would sacrifice the beautiful selector #sum for > the more intention revealing #sumNumbers. I think that makes sense. > > Concerning the #min and #max methods that means we’d end up with e.g. > #minNumbers, #min: and #min:ifEmpty: etc.
Yes. Doru > >> >> Cheers, >> Doru >> >> >>> On Dec 20, 2015, at 1:47 PM, Max Leske <maxle...@gmail.com> wrote: >>> >>>> >>>> On 20 Dec 2015, at 13:43, Gabriel Cotelli <g.cote...@gmail.com> wrote: >>>> >>>> Max, >>>> >>>> sum: aBlock ifEmpty: emptyBlock needs to obtain the sample evaluating the >>>> block. >>>> >>>> sum: aBlock ifEmpty: emptyBlock >>>> | sum sample | >>>> self isEmpty ifTrue: [ ^ emptyBlock value ]. >>>> sample := aBlock value: self anyOne. >>>> sum := self >>>> inject: sample >>>> into: [ :accum :each | accum + (aBlock value: each) ]. >>>> ^ sum - sample >>> >>> >>> Thanks! Missed that. >>> >>>> >>>> On Sun, Dec 20, 2015 at 8:59 AM, Max Leske <maxle...@gmail.com> wrote: >>>> I would like to wrap up this discussion. >>>> >>>> >>>>> On 05 Dec 2015, at 18:14, stepharo <steph...@free.fr> wrote: >>>>> >>>>> So what is the conclusion? >>>>> I like the idea of Esteban M to have iterator because it moves some >>>>> behavior out of core classes. >>>>> >>>>> [[[ >>>>> >>>>> aCollection arithmetic sum: [...] or.... aCollection >>>>> arithmetic avg. >>>>> >>>>> ]]] >>>>> >>>> >>>> While I think that iterators are an intriguing idea I also believe that >>>> they are beyond the scope of this issue. If anybody wants to follow up on >>>> iterators (or unit types for that matter) please start a new thread / >>>> issue. >>>> >>>> >>>> I propose to use Sven’s version for #sum:ifEmpty:. The result would be >>>> these three methods: >>>> >>>> sum >>>> ^ self >>>> sum: [ :each | each ] >>>> ifEmpty: [ 0 ] >>>> >>>> sum: aBlock >>>> ^ self >>>> sum: aBlock >>>> ifEmpty: [ self errorEmptyCollection ] >>>> >>>> sum: aBlock ifEmpty: emptyBlock >>>> | sum sample | >>>> self isEmpty ifTrue: [ ^ emptyBlock value ]. >>>> sample := self anyOne. >>>> sum := self >>>> inject: sample >>>> into: [ :accum :each | accum + (aBlock value: each) ]. >>>> ^ sum - sample >>>> >>>> >>>> I’ve attached a couple of benchmark results below. To me they show that >>>> 1. the new implementation is maybe a tiny bit slower but insignificantly >>>> so (if you’re going for performance you’ll probably write your own >>>> optimised version anyway) >>>> 2. there is no need to duplicate the code (like #sum and #sum: currently >>>> do). It’s perfectly fine to delegate to #sum:ifEmpty: >>>> >>>> >>>> >>>> In addition to the above changes I would like to remove #detectSum: (-> >>>> #sum:) and #sumNumbers (-> #sum). >>>> >>>> >>>> Note that once we agree on changing this API, we will need to also change >>>> #detectMin:, #detectMax:, #min, #max as well as all overrides (e.g. >>>> RunArray, Interval) of these and of #sum et. al. to stay consistent. The >>>> changes would of course be in line with this change, such that every >>>> operation has a unary selector with a sensible default, one that takes a >>>> block and throws an error for empty collections and a third that takes a >>>> block for the iteration and one for the empty case. >>>> >>>> >>>> Please cast your vote for these changes: >>>> >>>> 1. Do you agree to changing #sum and #sum: in the suggested way? >>>> >>>> 2. Do you agree to the removal of #detectSum:? >>>> >>>> 3. Do you agree to the removal of #sumNumbers? >>>> >>>> 4. Do you agree that the #max and #min selectors also need to be adapted? >>>> >>>> >>>> >>>> Thanks for you help. >>>> >>>> Cheers, >>>> Max >>>> >>>> >>>> >>>> >>>> >>>> Benchmarks >>>> ============ >>>> (Note that these aren’t very good benchmarks. There’s quite some variation >>>> on each run.) >>>> >>>> >>>> Machine: >>>> MacBook Pro (15-inch, Early 2011) >>>> CPU: 2.2 GHz Intel Core i7 >>>> Memory: 8 GB 1333 MHz DDR3 >>>> Disk: APPLE SSD TS512C (500 GB) >>>> >>>> >>>> Benchmarks of the current versions: >>>> >>>> [ (1 to: 1000000) asArray sum ] benchFor: 10 seconds. >>>> 75 iterations, 7.470 per second >>>> >>>> [ (1 to: 1000000) asArray sum: [ :e | e ] ] benchFor: 10 seconds. >>>> 72 iterations, 7.128 per second >>>> >>>> >>>> [ (1 to: 100) asArray sum ] benchFor: 10 seconds. >>>> 1,189,477 iterations, 118,912 per second >>>> >>>> >>>> [ (1 to: 100) asArray sum: [ :e | e ] ] benchFor: 10 seconds. >>>> 1,171,467 iterations, 117,112 per second >>>> >>>> >>>> >>>> Benchmarks of the new versions: >>>> >>>> [ (1 to: 1000000) asArray sum ] benchFor: 10 seconds. >>>> 73 iterations, 7.244 per second >>>> >>>> [ (1 to: 1000000) asArray sum: [ :e | e ] ] benchFor: 10 seconds. >>>> 75 iterations, 7.480 per second >>>> >>>> [ (1 to: 1000000) asArray sum: [ :e | e ] ifEmpty: [ 0 ] ] benchFor: 10 >>>> seconds. >>>> 72 iterations, 7.141 per second >>>> >>>> >>>> [ (1 to: 100) asArray sum ] benchFor: 10 seconds. >>>> 1,115,827 iterations, 111,560 per second >>>> >>>> [ (1 to: 100) asArray sum: [ :e | e ] ] benchFor: 10 seconds. >>>> 1,154,595 iterations, 115,425 per second >>>> >>>> [ (1 to: 100) asArray sum: [ :e | e ] ifEmpty: [ 0 ] ] benchFor: 10 >>>> seconds. >>>> 1,102,358 iterations, 110,203 per second >> >> -- >> www.tudorgirba.com >> www.feenk.com >> >> "There are no old things, there are only old ways of looking at them." >> >> >> >> >> > > -- www.tudorgirba.com www.feenk.com "If you can't say why something is relevant, it probably isn't."
