I would suggest not to change anything for now, let's wait for Max's poll or feedback from others. We could also pick this up again when we see each other IRL, like at the Pharo Days, that will certainly make it easier.
> On 20 Dec 2015, at 22:09, Tudor Girba <[email protected]> wrote: > > Hi, > > I cannot seem to agree with the current line of reasoning, so I am > withdrawing from this debate. It’s late in the year and I know I need a > break, so it is likely that I am missing something obvious or that I am just > persisting in some sort of bike-shedding point of view :). > > Thanks for keeping up with this. > > I wish you all a lovely holiday and see you next year! > > Doru > > >> On Dec 20, 2015, at 8:23 PM, Max Leske <[email protected]> wrote: >> >> I did a pass on all the changes that would be required (whatever the outcome >> of this discussion). Looks easy enough. One interesting point: >> FloatArray>>sum is implemented as a primitive in the FloatArrayPlugin and >> the zero element is explicitly defined as 0.0. The primitive will not fail >> for an empty collection but return 0.0. That would be in line with our new >> definition of #sum. >> >> >> One other thing: should the old selectors be marked as deprecated rather >> than being removed? I think that’s the general policy, right? >> >> Cheers, >> Max >> >> >>> On 20 Dec 2015, at 18:19, Max Leske <[email protected]> wrote: >>> >>> >>>> On 20 Dec 2015, at 15:26, Ben Coman <[email protected]> wrote: >>>> >>>> On Mon, Dec 21, 2015 at 12:43 AM, 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. >>>> >>>> >>>> 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 <[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? >>>> 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) >>> >> > > -- > www.tudorgirba.com > www.feenk.com > > "Speaking louder won't make the point worthier." > >
