Hi, I am not sure I understand the meaning of NO in the context of the previous message.
Is the NO related to the sumNumbers, or to the min, max? Cheers, Doru > On Dec 20, 2015, at 2:43 PM, Sven Van Caekenberghe <[email protected]> wrote: > > NO > >> On 20 Dec 2015, at 14:32, Tudor Girba <[email protected]> wrote: >> >> Hi, >> >>> On Dec 20, 2015, at 2:30 PM, Max Leske <[email protected]> wrote: >>> >>> >>>> On 20 Dec 2015, at 14:10, Tudor Girba <[email protected]> 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 <[email protected]> wrote: >>>>> >>>>>> >>>>>> On 20 Dec 2015, at 13:43, Gabriel Cotelli <[email protected]> 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 <[email protected]> wrote: >>>>>> I would like to wrap up this discussion. >>>>>> >>>>>> >>>>>>> On 05 Dec 2015, at 18:14, stepharo <[email protected]> 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." > > -- www.tudorgirba.com www.feenk.com "Beauty is where we see it."
