[Cuis-dev] [Integrated] Re: Fix for SequentiableCollection>>combinations:atATimeDo:

[Luciano Notarfrancesco]

Great! Thanks!

On Sat, May 25, 2019 at 8:31 PM Andres Valloud <ten at smallinteger.com> wrote:

> How about >20% faster in general?  I also fixed up comments that
> referenced methods and variables that didn't exist, etc.
>
> I strongly suspect this iteration can be made to run faster still.
>
> On 5/25/19 11:19, Luciano Notarfrancesco via Cuis-dev wrote:
> > Hi Hernan,
> > Thanks for integrating! Next time I should provide tests...
> > Yes, it is expected behavior, it was made like that in order to
> > enumerate combinations as fast as possible without stressing the GC (I
> > wonder how much difference does it make nowadays, tho). I thought that
> > the name 'atATimeDo' was a remainder that the array is being reused in
> > each iteration, but actually groupsOf:atATimeDo: doesn't reuse the
> > array, and permutationsDo: reuses the array. IMHO groupsOf:do: and
> > combinationsOf:do: would be nicer names, but not a big deal.
> >
> >
> > On Sat, May 25, 2019 at 1:06 PM Hernan Wilkinson
> > <hernan.wilkinson at 10pines.com <mailto:hernan.wilkinson at 10pines.com>>
> wrote:
> >
> >     Hi Luciano,
> >       thank you for the fix. It is integrated now and I added a couple
> >     of tests to SequenceableCollectionTest, one when k is cero and
> >     another for the normal case.
> >
> >       When I wrote the test for the normal case I notice that the
> >     collection passed as parameter to the block it is always the name
> >     and therefore the following test fails:
> >     testCombinationsAtATimeDoWorksAsExpected
> >
> >     | combinations |
> >
> >     combinations := OrderedCollection new.
> >     'abc' combinations: 2 atATimeDo: [ :combination | combinations add:
> >     combination].
> >
> >     self assert: 3 equals: combinations size.
> >     self assert: (combinations includes: #($a $b)).
> >     self assert: (combinations includes: #($a $c)).
> >     self assert: (combinations includes: #($b $c)).
> >
> >     To make it pass I had to make a copy of combination.
> >     ...
> >     'abc' combinations: 2 atATimeDo: [ :combination | combinations add:
> >     combination *copy*].
> >     ....
> >
> >     Is that the expected behavior? It looks weird to me... I would
> >     expect combination to be different collections on each iteration...
> >
> >     Cheers!
> >     Hernan.
> >
> >     On Thu, May 23, 2019 at 1:13 PM Luciano Notarfrancesco via Cuis-dev
> >     <cuis-dev at lists.cuis.st <mailto:cuis-dev at lists.cuis.st>> wrote:
> >
> >         The method was failing for the corner case of "combinations of 0
> >         elements". Here's the fix.
> >         --
> >         Cuis-dev mailing list
> >         Cuis-dev at lists.cuis.st <mailto:Cuis-dev at lists.cuis.st>
> >         https://lists.cuis.st/mailman/listinfo/cuis-dev
> >
> >
> >
> >     --
> >     *Hernán Wilkinson
> >     Agile Software Development, Teaching & Coaching*
> >     *Phone: +54-011*-4893-2057
> >     *Twitter: @HernanWilkinson*
> >     *site: http://www.10Pines.com <http://www.10pines.com/>*
> >     Address: Alem 896, Floor 6, Buenos Aires, Argentina
> >
> >
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.cuis.st/mailman/archives/cuis-dev/attachments/20190526/76e3e315/attachment-0001.html>