2015-12-01 11:46 GMT+01:00 Sven Van Caekenberghe <[email protected]>:
> The basic question for me is, what should
>
> #() sum
>
> return. Right now, it is an error, I would very much like that for this
> common case the result would be 0. There is a lot of power (easy of use) in
> a unary selector, we should not destroy that with semantics that force a
> test before using it.
>
Yes, this is just that a 0 value is returned for an empty collection, and,
if you're expecting a sum of Colors, then this is a bit strange.
Returning an error on a sum for an empty collection is maybe an overall
cleaner concept
Which can be expressed as:
I prefer this code:
aCollectionOfColors isEmpty
ifTrue: [ sumOfColors := aDefaultColor ]
ifFalse: [ sumOfColors := aCollectionOfColors sum ]
To this code:
sumOfColors := aCollectionOfColors sum.
sumOfColors = 0
ifTrue: [ sumOfColors := aDefaultColor ]
>
> If we agree on that, we could change the #sum to
>
> | sum sample |
> self isEmpty ifTrue: [ ^ 0 ].
> sample := self anyOne.
> sum := self inject: sample into: [ :accum :each | accum + each ].
> ^ sum - sample
>
Ok, now I really like this implementation. Elegant.
Thierry
>
> And the same for #sum:
>
> As for #detectSum: that should be removed I think.
>
> > On 01 Dec 2015, at 10:33, Nicolai Hess <[email protected]> wrote:
> >
> >
> >
> > 2015-12-01 10:26 GMT+01:00 Sven Van Caekenberghe <[email protected]>:
> >
> > > On 01 Dec 2015, at 10:18, Ben Coman <[email protected]> wrote:
> > >
> > > On Tue, Dec 1, 2015 at 4:45 PM, Sven Van Caekenberghe <[email protected]>
> wrote:
> > >> I am all for a cleanup, the current situation is confusing.
> > >> The basic #sum should be fast AND work for empty collections with 0
> as starting element.
> > >
> > >> I know why the #anyOne is used, and that use case should be
> preserved, but it is less common IMHO.
> > >
> > > I'm curious to be learn why?
> >
> > How many times have you summed a collection of things, where (1) you
> could not start from the number 0 and (2) the operation was #+ ? Me, never,
> and if I would need that, I would do an #inject:into: myself. Any counter
> examples that occur a lot ?
> >
> > wokring with things like Color
> >
> > (Color wheel:10) sum "works"
> > (Color wheel:10) newSum:#yourself " would not work"
> >
> >
> >
> > > cheers -ben
> > >
> > >>
> > >>> On 01 Dec 2015, at 09:38, Thierry Goubier <[email protected]>
> wrote:
> > >>>
> > >>> Hi Max,
> > >>>
> > >>> Interesting results...
> > >>>
> > >>> replacing the #yourself in newSum: cuts the runtime by a factor of
> two.
> > >>> using an [:x | x ] block cost 10% compared to a direct version of
> sum.
> > >>>
> > >>> So the fastest sum is:
> > >>>
> > >>> sum
> > >>> ^ self
> > >>> inject:
> > >>> (self ifEmpty: [ ^ 0 ] ifNotEmpty: [ self
> anyOne ])
> > >>> into: [ :sum :each | sum + each ]
> > >>>
> > >>> And
> > >>>
> > >>> sum
> > >>> ^ self sum: [:x | x ]
> > >>>
> > >>> is 10% slower.
> > >>>
> > >>> And
> > >>>
> > >>> sum
> > >>> ^ self sum: #yourself
> > >>>
> > >>> is 100% slower (i.e. x2)
> > >>>
> > >>> It may be wise to avoid using #yourself for a block.
> > >>>
> > >>> Thierry
> > >>>
> > >>>
> > >>> 2015-12-01 9:17 GMT+01:00 Max Leske <[email protected]>:
> > >>> Hi guys,
> > >>>
> > >>> Collection defines #sum:, #detectSum: and #sumNumbers:, all of which
> accomplish the same (in principal) but with subtle differences:
> > >>>
> > >>> #sum:
> > >>> - uses #inject:into: with #anyOne as the injected element and will
> thus fail for empty collections
> > >>>
> > >>> #detectSum:
> > >>> - uses “nextValue + sum” instead of “sum + nextValue” which makes it
> a lot slower when dealing with large numbers (primitive fails and number
> conversion is necessary)
> > >>>
> > >>> #sumNumbers:
> > >>> - same as #sum but doesn’t fail for empty collections. Only good for
> numbers though.
> > >>>
> > >>>
> > >>> Benchmarks:
> > >>>
> > >>> [ 100 timesRepeat: [ (1 to: 1000000) sum: #yourself ] ] timeToRun
> 18062
> > >>> [ 100 timesRepeat: [ (1 to: 1000000) detectSum: #yourself ] ]
> timeToRun 42391
> > >>> [ 100 timesRepeat: [ (1 to: 1000000) sumNumbers: #yourself ] ]
> timeToRun 18096
> > >>>
> > >>>
> > >>>
> > >>> Can we settle for a single implementation? Such as (modified from
> #sumNumbers:):
> > >>>
> > >>> newSum: aBlock
> > >>> ^ self
> > >>> inject: (self
> > >>> ifEmpty: [ 0 ]
> > >>> ifNotEmpty: [ self anyOne ])
> > >>> into: [ :sum :each | sum + (aBlock value: each) ]
> > >>>
> > >>> This implementation combines the best of the three implementations I
> think. Benchmark:
> > >>>
> > >>> [ 100 timesRepeat: [ (1 to: 1000000) newSum: #yourself ] ] timeToRun
> 17955
> > >>>
> > >>>
> > >>> BTW, there is also the message #sum, which suffers from the same
> problem as #sum: (the implementation is basically a copy). #sum could be
> implemented as
> > >>>
> > >>> sum
> > >>> ^ self sum: [ :x | x ]
> > >>>
> > >>>
> > >>> As for the name for the new method, I suggest #sum:.
> > >>>
> > >>> Cheers,
> > >>> Max
> > >>>
> > >>
> > >>
> > >
> >
> >
> >
>
>
>
