> On 20 Dec 2015, at 15:26, Ben Coman <b...@openinworld.com> wrote: > > On Mon, Dec 21, 2015 at 12:43 AM, Sven Van Caekenberghe <s...@stfx.eu > <mailto:s...@stfx.eu>> 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 <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 agree somewhat with Duro to avoid #sum and #sum: having different > semantics, > but I agree more with Sven about keeping #sum for numbers only and > returning zero, > so why not turn Doru's suggestion around... > > #sum (empty --> 0) > #sumBy: (empty --> error) > #sumBy: ifEmpty: (empty --> no error) > > alternatively could be #sumUsing: or #sumWith: or something similar.
Also a good idea. However, looking at the methods on Collection, the pattern is usually #message and #message:, e.g. #sorted and #sorted:. There used to be #sortBy: but it was removed because the view was that the protocol semantics should be the same across all methods. The current case is a bit different of course since we’re dealing with the problem of the zero element which doesn’t arise in most other cases. Still, I think from a users point of view it would be strange to have to use #sumBy: when every other message pair uses the #message / #message: pattern. I feel that this argument (thanks Ben :) ) is also a valid point against Doru’s argument for #sumNumbers. While #sumNumbers may *technically* be the best name (which we can’t seem to agree on…), #sum, #sum: and #sum:ifEmpty: form a triple that naturally fits into the naming protocol applied elsewhere. Cheers, Max > > > >>> >>> 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? > yes > >>>>> 2. Do you agree to the removal of #detectSum:? > ?? (not familiar) > >>>>> 3. Do you agree to the removal of #sumNumbers? > yes > >>>>> 4. Do you agree that the #max and #min selectors also need to be adapted? > ?? (not familiar)
