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."
