Hi, > On Dec 20, 2015, at 3:07 PM, Sven Van Caekenberghe <[email protected]> wrote: > > >> On 20 Dec 2015, at 14:57, Tudor Girba <[email protected]> wrote: >> >> Hi, >> >> It’s clear that choices we would make would not be the same :). So, let’s >> drop past discussions and stay with the current situation. Do you agree with >> the observation that in the proposal of Max #sum and #sum: would not share >> the same meaning? If yes, do you agree that it would be misleading from the >> outside? > > The overal meaning is the same, IMO, summing a number of things (most often > numbers) using #+. The processing block just adds functionality. > > The behaviour in the case of an empty collection is different yes. I want 0, > why, because of https://en.wikipedia.org/wiki/Empty_sum > > This is about optimisation for the common case.
Fair enough. But, then why are you not equally picking on the fact that #sum: throws an error? :) We are really overusing a term for something we should not and I think in the process we are getting to a tradeoff that does not lead to simplicity. From my point of view this discussion is similar to having Object>>#name. It kind of makes common sense to ask an object for its name, only that in a few situations it is actually painful to have it there :) > VW had no #sum, I think because of all the points raised in this discussion > (you can't always expect numbers and it is not clear what to do with empty > collections). I would not guide our decisions based on what VW did or did not do (I could not resist :)). > Check out the senders of #sum and #sum: in a 50 image. Sure, but we are supposed to create a language that is used by others. In the base image, there is no problem with Object>>#name either, but it is still not a good thing :) So, why not remove #sum altogether then? Cheers, Doru >> I would be fine with either answer. I would just want to understand. >> >> Cheers, >> Doru >> >> >>> On Dec 20, 2015, at 2:43 PM, Sven Van Caekenberghe <[email protected]> wrote: >>> >>> Doru, >>> >>> For me this whole discussion started because you (the standpoint that you >>> take) hijacked the best selector (#sum) for a use case that is much less >>> common than adding a collection of numbers which should give 0 when empty >>> (you hijack it by not wanting to return 0, which I totally understand, but >>> doing so you redefine the meaning/semantics). >>> >>> Max's point is to make everything more consistent and remove some API. I >>> support that too. >>> >>> Now, I like Max's proposal. >>> >>> But, you know, my conclusion when the thread ended was that it might be >>> better to throw all these selectors away, since #inject:into: covers most >>> use cases at least as well and at least as clear. >>> >>> Sven >>> >>>> 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. >>>> >>>> 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 >> >> "It's not how it is, it is how we see it." >> >> > > -- www.tudorgirba.com www.feenk.com "What we can governs what we wish."
