On Mon, Dec 21, 2015 at 12:43 AM, Sven Van Caekenberghe <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. >> >> 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)
