[Pharo-users] Set >> collect:thenDo:

Steffen Märcker merkste at web.de
Mon Sep 9 09:50:18 EDT 2019


I think this thread indicates that the selector #collect:thenDo: may
indeed cause some missunderstanding given standard semantics of #collect:.
Is there any reason not to use a different one, such as #map:thenDo:, to
make the difference clearer?

A nice side effect could be a new automatic refactoring rule that
consolidates chained #collect: and #do: sends into a faster
#collect:thenDo: send without breaking existing code.

Am .09.2019, 07:46 Uhr, schrieb Richard O'Keefe <raoknz at gmail.com>:

> (1) If you want (aSet collect: collectBlock) do: doBlock
>     you can write exactly that.  Nothing stops you, and it will
>     be as clear and reliable as any use of Set>>collect:, which
>     is to say NOT VERY CLEAR OR RELIABLE AT ALL.
>
> (2) #collect:then{Do:Select:Reject:} had no other purpose than
>     to avoid creating an intermediate and otherwise useless
>     collection.  If you are not trying to involve an intermediate
>     set then it just plain wrong to use #collect:thenDo:.
>
> (3) Oddly enough, the reason that #collect:thenDo: exists in my
>     library is that I copied it from Squeak, at a time when it had
>     the same definition as Pharo and ST/X.  Had I known of the change
>     in Squeak I would have bitterly opposed it.  The comment in the
>     current Squeak code, that it is for readability, is 100% the
>     reverse of the truth.  Using the version with parentheses is WAY
>     clearer than using the portmanteau method.  Sigh.  I see they
>     broke #collect:thenSelect: in the same way.
>
> (4) Let me offer you another member of the #collect:then* family.
>
>     Collection
>       collect: collectBlock thenInject: initial into: injectBlock
>         |r|
>         r := initial.
>         self do: [:each |
>           r := injectBlock value: r value: (collectBlock value: each)].
>         ^r
>
>     #(now is the hour for us to say goodbye) asSet
>       collect: [:each | each size]
>       thenInject: 0 into: [:acc :each | acc + each]
>  => 29
>     (#(now is the hour for us to say goodbye) asSet
>       collect: [:each | each size])
>       inject: 0 into: [:acc :each | acc + each]
>  => 16
>
>    That is, it would be WRONG to implement #collect:thenInject:into:
>    as #collect: followed by #inject:into:.  The point is NOT to
>    coalesce things that the receiver might (in general, incorrectly)
>    regard as equal.
>
> (5) The bottom line is that if #collect:thenDo: and its relatives did
>     not have their present semantics in Pharo (and ST/X), they would
>     need to be reinvented, with names that were not as clear.
>
> (1) Just to repeat for emphasis, if you *want* (_ collect: _) do: _
>     then that is exactly what you should write.  There is no
>     excuse for using #collect:thenDo: in that case.  It is NOT
>     clearer to do so.  And you should imagine me jumping up and
>     down screaming that sending #collect: to a set is a bad code
>     smell which demands very explicit documentation as to why you
>     (for example) want a Set to answer a Set but an IdentitySet
>     to also answer a Set, not the expected IdentitySet.  (I have
>     been bitten by that more than once.)
>
>
>
> On Mon, 9 Sep 2019 at 01:33, Herby Vojčík <herby at mailbox.sk> wrote:
>
>> On 8. 9. 2019 14:28, Peter Kenny wrote:
>> > Two comments:
>> > First, the method comment for Collection>>collect:thenDo: is "Utility
>> method
>> > to improve readability", which is exactly the same as for
>> > collect:thenSelect: and collect:thenReject:. This suggests that the
>> > *intention* of the method is not to introduce new behaviour, but
>> simply
>> to
>> > provide a shorthand for the version with parentheses. For other kinds
>> of
>>
>> I had that same impression.
>>
>> > collection this is true; just the deduping makes Set different. If we
>> want
>>
>> I would be more defensive here and say that generic collection should
>> have (collect:)do: implementation and only sequenceable collections have
>> the optimized one (if it indeed is the case that it is a shotrhand for
>> parenthesized one).
>>
>> > the different behaviour, this should be indicated by method name and
>> > comment.
>> > Second, if we remove asSet from the second snippet, the output is
>> exactly
>> > the same. It will be the same as long as the original collection has
>> no
>> > duplicates. Somehow the effect is to ignore the asSet. It just smells
>> wrong.
>> >
>> > Peter Kenny
>>
>> Herby
>>
>> > Kasper Osterbye wrote
>> >> The first version:
>> >>
>> >> (#(1 2 3) asSet collect: #odd)
>> >> do: [ :each | Transcript show: each; cr ]
>> >>
>> >> is rather straight forward I believe, as collect: and do: has been
>> around
>> >> forever (relatively speaking).
>> >>
>> >>
>> >> #(1 2 3) asSet collect: #odd
>> >> thenDo: [ :each | Transcript show: each; cr ]
>> >>
>> >>
>> >> On 8 September 2019 at 09.13.36, Richard Sargent (
>> >
>> >> richard.sargent@
>> >
>> >> ) wrote:
>> >>
>> >>   I am skeptical of one that relies on a specific implementation
>> rather
>> >> than
>> >> a specific definition.
>> >>
>> >> I share your feeling. I am not sure where such a definition would
>> come
>> >> from. In Squeak it is defined as:
>> >>
>> >> collect: collectBlock thenDo: doBlock
>> >>
>> >> ^ (self collect: collectBlock) do: doBlock
>> >>
>> >> In pharo as:
>> >>
>> >> collect: collectBlock thenDo: doBlock
>> >>
>> >> ^ self do: [ :each | doBlock value: (collectBlock value: each)]
>> >>
>> >> I might have called the method collect:eachDo:, but we each have
>> slightly
>> >> different styles. What I like about the pharo version is that is a
>> >> shorthand for something which is not achieved by mere parentheses.
>> >>
>> >> Best,
>> >>
>> >> Kasper
>> >
>> >
>> >
>> >
>> >
>> > --
>> > Sent from: http://forum.world.st/Pharo-Smalltalk-Users-f1310670.html
>> >
>>
>>
>>





More information about the Pharo-users mailing list