On 20 Dec 2015, at 15:26, Ben Coman <btc@openInWorld.com> wrote:

On Mon, Dec 21, 2015 at 12:43 AM, Sven Van Caekenberghe <sven@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 <tudor@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 <maxleske@gmail.com> wrote:

On 20 Dec 2015, at 13:43, Gabriel Cotelli <g.cotelli@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 <maxleske@gmail.com> wrote:
I would like to wrap up this discussion.

On 05 Dec 2015, at 18:14, stepharo <stepharo@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)