On 12/01, Thierry Goubier wrote:
> 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 ]
or
sumOfColors := (aCollectionOfColors ifEmpty: [ {aDefaultColor} ]) sum.
>
>
> >
> > 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.
Why the temp variables?
self ifEmpty: [ ^ 0 ].
^ self inject: self anyOne negated into: [ :sum :each | sum + each ]
But more importantly, if the empty/default is 0 then what is the reason for
#anyOne?
Otherwise you are mixing things.
self ifEmpty: [ ^ 0 ].
^ self inject: 0 into: [ :sum :each | sum + each ].
Peter
>
> 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
> > > >>>
> > > >>
> > > >>
> > > >
> > >
> > >
> > >
> >
> >
> >
--
Peter
