The Trunk: Collections-eem.756.mcz

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
25 messages Options
12
Reply | Threaded
Open this post in threaded view
|

The Trunk: Collections-eem.756.mcz

commits-2
Eliot Miranda uploaded a new version of Collections to project The Trunk:
http://source.squeak.org/trunk/Collections-eem.756.mcz

==================== Summary ====================

Name: Collections-eem.756
Author: eem
Time: 14 June 2017, 11:03:24.917631 am
UUID: 8d7c03bc-1cdb-44c7-9173-10d50c0dae29
Ancestors: Collections-eem.755

Add SequenceableCollection>>withoutDuplicates for a more elegant fix to MailMessage>>to:

=============== Diff against Collections-eem.755 ===============

Item was added:
+ ----- Method: SequenceableCollection>>withoutDuplicates (in category 'copying') -----
+ withoutDuplicates
+ "Answer a copy of the receiver that preserves order but eliminates any duplicates."
+ | seen |
+ seen := Set new: self size.
+ ^self select: [:each|
+  (seen includes: each)
+ ifTrue: [false]
+ ifFalse: [seen add: each. true]]!


Reply | Threaded
Open this post in threaded view
|

Re: The Trunk: Collections-eem.756.mcz

Levente Uzonyi
On Wed, 14 Jun 2017, [hidden email] wrote:

> Eliot Miranda uploaded a new version of Collections to project The Trunk:
> http://source.squeak.org/trunk/Collections-eem.756.mcz
>
> ==================== Summary ====================
>
> Name: Collections-eem.756
> Author: eem
> Time: 14 June 2017, 11:03:24.917631 am
> UUID: 8d7c03bc-1cdb-44c7-9173-10d50c0dae29
> Ancestors: Collections-eem.755
>
> Add SequenceableCollection>>withoutDuplicates for a more elegant fix to MailMessage>>to:
>
> =============== Diff against Collections-eem.755 ===============
>
> Item was added:
> + ----- Method: SequenceableCollection>>withoutDuplicates (in category 'copying') -----
> + withoutDuplicates
> + "Answer a copy of the receiver that preserves order but eliminates any duplicates."
> + | seen |
> + seen := Set new: self size.
> + ^self select: [:each|
> +  (seen includes: each)
> + ifTrue: [false]
> + ifFalse: [seen add: each. true]]!

What a great opportunity to use #addNewElement::

  ^self select: [ :each | seen addNewElement: each ]

Levente

Reply | Threaded
Open this post in threaded view
|

Re: The Trunk: Collections-eem.756.mcz

Eliot Miranda-2
Hi Levente, Hi Chris,

On Wed, Jun 14, 2017 at 1:10 PM, Levente Uzonyi <[hidden email]> wrote:
On Wed, 14 Jun 2017, [hidden email] wrote:

Eliot Miranda uploaded a new version of Collections to project The Trunk:
http://source.squeak.org/trunk/Collections-eem.756.mcz

==================== Summary ====================

Name: Collections-eem.756
Author: eem
Time: 14 June 2017, 11:03:24.917631 am
UUID: 8d7c03bc-1cdb-44c7-9173-10d50c0dae29
Ancestors: Collections-eem.755

Add SequenceableCollection>>withoutDuplicates for a more elegant fix to MailMessage>>to:

=============== Diff against Collections-eem.755 ===============

Item was added:
+ ----- Method: SequenceableCollection>>withoutDuplicates (in category 'copying') -----
+ withoutDuplicates
+       "Answer a copy of the receiver that preserves order but eliminates any duplicates."
+       | seen |
+       seen := Set new: self size.
+       ^self select: [:each|
+                                 (seen includes: each)
+                                       ifTrue: [false]
+                                       ifFalse: [seen add: each. true]]!

What a great opportunity to use #addNewElement::

        ^self select: [ :each | seen addNewElement: each ]

I love the functionality but I don't like the selector. It seems to imply that one must only add a new element.  So why not call this something like addIfAbsent: ?

Here are some suggestions.  Votes?
- don't change it; stick with addNewElement:
- addIfAbsent:
- ifAbsentAdd:
- ifMissingAdd:

I think I prefer ifAbsentAdd: cuz addIfAbsent: looks too much like a potential spelling error, and conflicts with typical ifAbsent: arguments supplying exception blocks.  But I could go with ifMissingAdd: because it is more distinctive.
 
Levente

_,,,^..^,,,_
best, Eliot


Reply | Threaded
Open this post in threaded view
|

Re: The Trunk: Collections-eem.756.mcz

Chris Muller-4
Hi Eliot,

Yes, because of the dual purpose of this method; 1) to ensure presence of an element and, 2) answer whether the element already existed, there isn't a clear and concise name which conveys the full nuance.  It's one of those methods which requires an initial look at the comment and/or code until one is familiar with it.

I considered those names, too.  I didn't care for addIfAbsent: for the reason you stated.  I don't care for ifAbsentAdd: because it's backward with other "Absent" nomenclature in the API.

And, I didn't go with #ifMissingAdd: because it confuses the return value with the linguistic semantics -- e.g., if it returns true, does that mean it was "absent" or that it was added?  forcing me to look at the method implementation again anyway.

#addNewElement: fits in with the #add... nomenclature part of the API, being like a "command" to the object, to which it may answer, "no" (false), if the element already existed.

It seems the only initial confusion you had was whether it signaled an Exception or not, but I see no other name that avoids the user needing to look at the implementation at least the first time.

Therefore, I think #addNewElement: is a good name.

Best,
  Chris

On Thu, Jun 15, 2017 at 1:30 PM, Eliot Miranda <[hidden email]> wrote:
Hi Levente, Hi Chris,

On Wed, Jun 14, 2017 at 1:10 PM, Levente Uzonyi <[hidden email]> wrote:
On Wed, 14 Jun 2017, [hidden email] wrote:

Eliot Miranda uploaded a new version of Collections to project The Trunk:
http://source.squeak.org/trunk/Collections-eem.756.mcz

==================== Summary ====================

Name: Collections-eem.756
Author: eem
Time: 14 June 2017, 11:03:24.917631 am
UUID: 8d7c03bc-1cdb-44c7-9173-10d50c0dae29
Ancestors: Collections-eem.755

Add SequenceableCollection>>withoutDuplicates for a more elegant fix to MailMessage>>to:

=============== Diff against Collections-eem.755 ===============

Item was added:
+ ----- Method: SequenceableCollection>>withoutDuplicates (in category 'copying') -----
+ withoutDuplicates
+       "Answer a copy of the receiver that preserves order but eliminates any duplicates."
+       | seen |
+       seen := Set new: self size.
+       ^self select: [:each|
+                                 (seen includes: each)
+                                       ifTrue: [false]
+                                       ifFalse: [seen add: each. true]]!

What a great opportunity to use #addNewElement::

        ^self select: [ :each | seen addNewElement: each ]

I love the functionality but I don't like the selector. It seems to imply that one must only add a new element.  So why not call this something like addIfAbsent: ?

Here are some suggestions.  Votes?
- don't change it; stick with addNewElement:
- addIfAbsent:
- ifAbsentAdd:
- ifMissingAdd:

I think I prefer ifAbsentAdd: cuz addIfAbsent: looks too much like a potential spelling error, and conflicts with typical ifAbsent: arguments supplying exception blocks.  But I could go with ifMissingAdd: because it is more distinctive.
 
Levente

_,,,^..^,,,_
best, Eliot



Reply | Threaded
Open this post in threaded view
|

Re: The Trunk: Collections-eem.756.mcz

Tobias Pape
In reply to this post by Eliot Miranda-2

> On 15.06.2017, at 20:30, Eliot Miranda <[hidden email]> wrote:
>
> Hi Levente, Hi Chris,
>
> On Wed, Jun 14, 2017 at 1:10 PM, Levente Uzonyi <[hidden email]> wrote:
> On Wed, 14 Jun 2017, [hidden email] wrote:
>
> Eliot Miranda uploaded a new version of Collections to project The Trunk:
> http://source.squeak.org/trunk/Collections-eem.756.mcz
>
> ==================== Summary ====================
>
> Name: Collections-eem.756
> Author: eem
> Time: 14 June 2017, 11:03:24.917631 am
> UUID: 8d7c03bc-1cdb-44c7-9173-10d50c0dae29
> Ancestors: Collections-eem.755
>
> Add SequenceableCollection>>withoutDuplicates for a more elegant fix to MailMessage>>to:
>
> =============== Diff against Collections-eem.755 ===============
>
> Item was added:
> + ----- Method: SequenceableCollection>>withoutDuplicates (in category 'copying') -----
> + withoutDuplicates
> +       "Answer a copy of the receiver that preserves order but eliminates any duplicates."
> +       | seen |
> +       seen := Set new: self size.
> +       ^self select: [:each|
> +                                 (seen includes: each)
> +                                       ifTrue: [false]
> +                                       ifFalse: [seen add: each. true]]!
>
> What a great opportunity to use #addNewElement::
>
>         ^self select: [ :each | seen addNewElement: each ]
>
> I love the functionality but I don't like the selector. It seems to imply that one must only add a new element.  So why not call this something like addIfAbsent: ?
>
> Here are some suggestions.  Votes?
> - don't change it; stick with addNewElement:
> - addIfAbsent:
> - ifAbsentAdd:
> - ifMissingAdd:
>
> I think I prefer ifAbsentAdd: cuz addIfAbsent: looks too much like a potential spelling error, and conflicts with typical ifAbsent: arguments supplying exception blocks.  But I could go with ifMissingAdd: because it is more distinctive.

Well, we do have

        Collection>>addIfNotPresent:

So why invent a new one?

I think the important thing with #addNewElement: is that it returns *whether* it added a new element
but then again it breaks the tradition of #add* returning its argument…

a very clear and very strange one that would reveal that a boolean is returned would be #isAbsentAndIfSoAdd: …

Best regards
        -Tobias



>
> _,,,^..^,,,_
> best, Eliot


Reply | Threaded
Open this post in threaded view
|

Re: The Trunk: Collections-eem.756.mcz

Eliot Miranda-2
In reply to this post by Chris Muller-4
Hi Chris,

On Thu, Jun 15, 2017 at 12:05 PM, Chris Muller <[hidden email]> wrote:
Hi Eliot,

Yes, because of the dual purpose of this method; 1) to ensure presence of an element and, 2) answer whether the element already existed,

to be pedantic, answer if the element didn't already exist
 
there isn't a clear and concise name which conveys the full nuance.  It's one of those methods which requires an initial look at the comment and/or code until one is familiar with it.

Given that addNewElement: doesn't convey the full nuance that's not a valid criticism of the other alternatives, is it ;-)

I considered those names, too.  I didn't care for addIfAbsent: for the reason you stated.  I don't care for ifAbsentAdd: because it's backward with other "Absent" nomenclature in the API.

And, I didn't go with #ifMissingAdd: because it confuses the return value with the linguistic semantics -- e.g., if it returns true, does that mean it was "absent" or that it was added?  forcing me to look at the method implementation again anyway.

#addNewElement: fits in with the #add... nomenclature part of the API, being like a "command" to the object, to which it may answer, "no" (false), if the element already existed.

It seems the only initial confusion you had was whether it signaled an Exception or not, but I see no other name that avoids the user needing to look at the implementation at least the first time.

No.  The selector implies it might.  The selector is bad because this does not add a new element, it ensures an element is present, and answers if it was added.
 

Therefore, I think #addNewElement: is a good name.

I beg to differ.  I find it misleading :-(  Which is a shame because it's really useful.  How about trying to reach consensus on a less misleading name?
 
Best,
  Chris

On Thu, Jun 15, 2017 at 1:30 PM, Eliot Miranda <[hidden email]> wrote:
Hi Levente, Hi Chris,

On Wed, Jun 14, 2017 at 1:10 PM, Levente Uzonyi <[hidden email]> wrote:
On Wed, 14 Jun 2017, [hidden email] wrote:

Eliot Miranda uploaded a new version of Collections to project The Trunk:
http://source.squeak.org/trunk/Collections-eem.756.mcz

==================== Summary ====================

Name: Collections-eem.756
Author: eem
Time: 14 June 2017, 11:03:24.917631 am
UUID: 8d7c03bc-1cdb-44c7-9173-10d50c0dae29
Ancestors: Collections-eem.755

Add SequenceableCollection>>withoutDuplicates for a more elegant fix to MailMessage>>to:

=============== Diff against Collections-eem.755 ===============

Item was added:
+ ----- Method: SequenceableCollection>>withoutDuplicates (in category 'copying') -----
+ withoutDuplicates
+       "Answer a copy of the receiver that preserves order but eliminates any duplicates."
+       | seen |
+       seen := Set new: self size.
+       ^self select: [:each|
+                                 (seen includes: each)
+                                       ifTrue: [false]
+                                       ifFalse: [seen add: each. true]]!

What a great opportunity to use #addNewElement::

        ^self select: [ :each | seen addNewElement: each ]

I love the functionality but I don't like the selector. It seems to imply that one must only add a new element.  So why not call this something like addIfAbsent: ?

Here are some suggestions.  Votes?
- don't change it; stick with addNewElement:
- addIfAbsent:
- ifAbsentAdd:
- ifMissingAdd:

I think I prefer ifAbsentAdd: cuz addIfAbsent: looks too much like a potential spelling error, and conflicts with typical ifAbsent: arguments supplying exception blocks.  But I could go with ifMissingAdd: because it is more distinctive.
 
Levente

_,,,^..^,,,_
best, Eliot




--
_,,,^..^,,,_
best, Eliot


Reply | Threaded
Open this post in threaded view
|

Re: The Trunk: Collections-eem.756.mcz

Eliot Miranda-2
In reply to this post by Tobias Pape
Hi Tobias,

On Thu, Jun 15, 2017 at 12:06 PM, Tobias Pape <[hidden email]> wrote:

> On 15.06.2017, at 20:30, Eliot Miranda <[hidden email]> wrote:
>
> Hi Levente, Hi Chris,
>
> On Wed, Jun 14, 2017 at 1:10 PM, Levente Uzonyi <[hidden email]> wrote:
> On Wed, 14 Jun 2017, [hidden email] wrote:
>
> Eliot Miranda uploaded a new version of Collections to project The Trunk:
> http://source.squeak.org/trunk/Collections-eem.756.mcz
>
> ==================== Summary ====================
>
> Name: Collections-eem.756
> Author: eem
> Time: 14 June 2017, 11:03:24.917631 am
> UUID: 8d7c03bc-1cdb-44c7-9173-10d50c0dae29
> Ancestors: Collections-eem.755
>
> Add SequenceableCollection>>withoutDuplicates for a more elegant fix to MailMessage>>to:
>
> =============== Diff against Collections-eem.755 ===============
>
> Item was added:
> + ----- Method: SequenceableCollection>>withoutDuplicates (in category 'copying') -----
> + withoutDuplicates
> +       "Answer a copy of the receiver that preserves order but eliminates any duplicates."
> +       | seen |
> +       seen := Set new: self size.
> +       ^self select: [:each|
> +                                 (seen includes: each)
> +                                       ifTrue: [false]
> +                                       ifFalse: [seen add: each. true]]!
>
> What a great opportunity to use #addNewElement::
>
>         ^self select: [ :each | seen addNewElement: each ]
>
> I love the functionality but I don't like the selector. It seems to imply that one must only add a new element.  So why not call this something like addIfAbsent: ?
>
> Here are some suggestions.  Votes?
> - don't change it; stick with addNewElement:
> - addIfAbsent:
> - ifAbsentAdd:
> - ifMissingAdd:
>
> I think I prefer ifAbsentAdd: cuz addIfAbsent: looks too much like a potential spelling error, and conflicts with typical ifAbsent: arguments supplying exception blocks.  But I could go with ifMissingAdd: because it is more distinctive.

Well, we do have

        Collection>>addIfNotPresent:

So why invent a new one?

Because addIfNotPresent: answers its argument and we need one that answers whether the element was absent.  So alas addIfNotPresent: is not a suitable candidate.


I think the important thing with #addNewElement: is that it returns *whether* it added a new element
but then again it breaks the tradition of #add* returning its argument…

a very clear and very strange one that would reveal that a boolean is returned would be #isAbsentAndIfSoAdd: …

wasAbsentAdding: or ifWasAbsentAdding: would be less cumbersome but I like something snappier that people will remember.  ifMissingAdd: looks good because it doesn't conflict with the add*: methods answering their argument, and the ifMissing implies the answer is true if the element wasn't already present.

Stéphane, can you live with ifMissingAdd: ?  Chris?
 
Best regards
        -Tobias

> _,,,^..^,,,_
> best, Eliot
 
_,,,^..^,,,_
best, Eliot


cbc
Reply | Threaded
Open this post in threaded view
|

Re: The Trunk: Collections-eem.756.mcz

cbc
#testMissingThenAdd:  ?

On Jun 15, 2017 12:19 PM, "Eliot Miranda" <[hidden email]> wrote:
Hi Tobias,

On Thu, Jun 15, 2017 at 12:06 PM, Tobias Pape <[hidden email]> wrote:

> On 15.06.2017, at 20:30, Eliot Miranda <[hidden email]> wrote:
>
> Hi Levente, Hi Chris,
>
> On Wed, Jun 14, 2017 at 1:10 PM, Levente Uzonyi <[hidden email]> wrote:
> On Wed, 14 Jun 2017, [hidden email] wrote:
>
> Eliot Miranda uploaded a new version of Collections to project The Trunk:
> http://source.squeak.org/trunk/Collections-eem.756.mcz
>
> ==================== Summary ====================
>
> Name: Collections-eem.756
> Author: eem
> Time: 14 June 2017, 11:03:24.917631 am
> UUID: 8d7c03bc-1cdb-44c7-9173-10d50c0dae29
> Ancestors: Collections-eem.755
>
> Add SequenceableCollection>>withoutDuplicates for a more elegant fix to MailMessage>>to:
>
> =============== Diff against Collections-eem.755 ===============
>
> Item was added:
> + ----- Method: SequenceableCollection>>withoutDuplicates (in category 'copying') -----
> + withoutDuplicates
> +       "Answer a copy of the receiver that preserves order but eliminates any duplicates."
> +       | seen |
> +       seen := Set new: self size.
> +       ^self select: [:each|
> +                                 (seen includes: each)
> +                                       ifTrue: [false]
> +                                       ifFalse: [seen add: each. true]]!
>
> What a great opportunity to use #addNewElement::
>
>         ^self select: [ :each | seen addNewElement: each ]
>
> I love the functionality but I don't like the selector. It seems to imply that one must only add a new element.  So why not call this something like addIfAbsent: ?
>
> Here are some suggestions.  Votes?
> - don't change it; stick with addNewElement:
> - addIfAbsent:
> - ifAbsentAdd:
> - ifMissingAdd:
>
> I think I prefer ifAbsentAdd: cuz addIfAbsent: looks too much like a potential spelling error, and conflicts with typical ifAbsent: arguments supplying exception blocks.  But I could go with ifMissingAdd: because it is more distinctive.

Well, we do have

        Collection>>addIfNotPresent:

So why invent a new one?

Because addIfNotPresent: answers its argument and we need one that answers whether the element was absent.  So alas addIfNotPresent: is not a suitable candidate.


I think the important thing with #addNewElement: is that it returns *whether* it added a new element
but then again it breaks the tradition of #add* returning its argument…

a very clear and very strange one that would reveal that a boolean is returned would be #isAbsentAndIfSoAdd: …

wasAbsentAdding: or ifWasAbsentAdding: would be less cumbersome but I like something snappier that people will remember.  ifMissingAdd: looks good because it doesn't conflict with the add*: methods answering their argument, and the ifMissing implies the answer is true if the element wasn't already present.

Stéphane, can you live with ifMissingAdd: ?  Chris?
 
Best regards
        -Tobias

> _,,,^..^,,,_
> best, Eliot
 
_,,,^..^,,,_
best, Eliot





Reply | Threaded
Open this post in threaded view
|

Re: The Trunk: Collections-eem.756.mcz

Tobias Pape

> On 15.06.2017, at 21:22, Chris Cunningham <[hidden email]> wrote:
>
> #testMissingThenAdd:  ?
>

#isAdding:
#ingests:

#wasAbsentButNowIsPresent:

(just rambling…)

> On Jun 15, 2017 12:19 PM, "Eliot Miranda" <[hidden email]> wrote:
> Hi Tobias,
>
> On Thu, Jun 15, 2017 at 12:06 PM, Tobias Pape <[hidden email]> wrote:
>
> > On 15.06.2017, at 20:30, Eliot Miranda <[hidden email]> wrote:
> >
> > Hi Levente, Hi Chris,
> >
> > On Wed, Jun 14, 2017 at 1:10 PM, Levente Uzonyi <[hidden email]> wrote:
> > On Wed, 14 Jun 2017, [hidden email] wrote:
> >
> > Eliot Miranda uploaded a new version of Collections to project The Trunk:
> > http://source.squeak.org/trunk/Collections-eem.756.mcz
> >
> > ==================== Summary ====================
> >
> > Name: Collections-eem.756
> > Author: eem
> > Time: 14 June 2017, 11:03:24.917631 am
> > UUID: 8d7c03bc-1cdb-44c7-9173-10d50c0dae29
> > Ancestors: Collections-eem.755
> >
> > Add SequenceableCollection>>withoutDuplicates for a more elegant fix to MailMessage>>to:
> >
> > =============== Diff against Collections-eem.755 ===============
> >
> > Item was added:
> > + ----- Method: SequenceableCollection>>withoutDuplicates (in category 'copying') -----
> > + withoutDuplicates
> > +       "Answer a copy of the receiver that preserves order but eliminates any duplicates."
> > +       | seen |
> > +       seen := Set new: self size.
> > +       ^self select: [:each|
> > +                                 (seen includes: each)
> > +                                       ifTrue: [false]
> > +                                       ifFalse: [seen add: each. true]]!
> >
> > What a great opportunity to use #addNewElement::
> >
> >         ^self select: [ :each | seen addNewElement: each ]
> >
> > I love the functionality but I don't like the selector. It seems to imply that one must only add a new element.  So why not call this something like addIfAbsent: ?
> >
> > Here are some suggestions.  Votes?
> > - don't change it; stick with addNewElement:
> > - addIfAbsent:
> > - ifAbsentAdd:
> > - ifMissingAdd:
> >
> > I think I prefer ifAbsentAdd: cuz addIfAbsent: looks too much like a potential spelling error, and conflicts with typical ifAbsent: arguments supplying exception blocks.  But I could go with ifMissingAdd: because it is more distinctive.
>
> Well, we do have
>
>         Collection>>addIfNotPresent:
>
> So why invent a new one?
>
> Because addIfNotPresent: answers its argument and we need one that answers whether the element was absent.  So alas addIfNotPresent: is not a suitable candidate.
>
>
> I think the important thing with #addNewElement: is that it returns *whether* it added a new element
> but then again it breaks the tradition of #add* returning its argument…
>
> a very clear and very strange one that would reveal that a boolean is returned would be #isAbsentAndIfSoAdd: …
>
> wasAbsentAdding: or ifWasAbsentAdding: would be less cumbersome but I like something snappier that people will remember.  ifMissingAdd: looks good because it doesn't conflict with the add*: methods answering their argument, and the ifMissing implies the answer is true if the element wasn't already present.
>
> Stéphane, can you live with ifMissingAdd: ?  Chris?
>  
> Best regards
>         -Tobias
>
> > _,,,^..^,,,_
> > best, Eliot
>  
> _,,,^..^,,,_
> best, Eliot
>
>
>
>


Reply | Threaded
Open this post in threaded view
|

Re: The Trunk: Collections-eem.756.mcz

Eliot Miranda-2
Hi Tobias,

On Thu, Jun 15, 2017 at 12:32 PM, Tobias Pape <[hidden email]> wrote:

> On 15.06.2017, at 21:22, Chris Cunningham <[hidden email]> wrote:
>
> #testMissingThenAdd:  ?
>

#isAdding:
#ingests:

#wasAbsentButNowIsPresent:

(just rambling…)

I like added:.  Its concise, accurate and reads nicely:

withoutDuplicates
"Answer a copy of the receiver that preserves order but eliminates any duplicates."
| seen |
seen := Set new: self size.
^self select: [:each| seen added: each]

messagesDo: aBlock
"Evaluate aBlock exactly once with all the message selectors sent by me."

| scanner aSet |
self isQuick ifTrue: [ ^self ].
scanner := InstructionStream on: self.
scanner scanFor: [ :x |
| selector |
(selector := scanner selectorToSendOrSelf) == scanner ifFalse: [
((aSet ifNil: [ aSet := IdentitySet new ]) added: selector) ifTrue: [
aBlock value: selector ] ].
false "keep scanning" ]

 

> On Jun 15, 2017 12:19 PM, "Eliot Miranda" <[hidden email]> wrote:
> Hi Tobias,
>
> On Thu, Jun 15, 2017 at 12:06 PM, Tobias Pape <[hidden email]> wrote:
>
> > On 15.06.2017, at 20:30, Eliot Miranda <[hidden email]> wrote:
> >
> > Hi Levente, Hi Chris,
> >
> > On Wed, Jun 14, 2017 at 1:10 PM, Levente Uzonyi <[hidden email]> wrote:
> > On Wed, 14 Jun 2017, [hidden email] wrote:
> >
> > Eliot Miranda uploaded a new version of Collections to project The Trunk:
> > http://source.squeak.org/trunk/Collections-eem.756.mcz
> >
> > ==================== Summary ====================
> >
> > Name: Collections-eem.756
> > Author: eem
> > Time: 14 June 2017, 11:03:24.917631 am
> > UUID: 8d7c03bc-1cdb-44c7-9173-10d50c0dae29
> > Ancestors: Collections-eem.755
> >
> > Add SequenceableCollection>>withoutDuplicates for a more elegant fix to MailMessage>>to:
> >
> > =============== Diff against Collections-eem.755 ===============
> >
> > Item was added:
> > + ----- Method: SequenceableCollection>>withoutDuplicates (in category 'copying') -----
> > + withoutDuplicates
> > +       "Answer a copy of the receiver that preserves order but eliminates any duplicates."
> > +       | seen |
> > +       seen := Set new: self size.
> > +       ^self select: [:each|
> > +                                 (seen includes: each)
> > +                                       ifTrue: [false]
> > +                                       ifFalse: [seen add: each. true]]!
> >
> > What a great opportunity to use #addNewElement::
> >
> >         ^self select: [ :each | seen addNewElement: each ]
> >
> > I love the functionality but I don't like the selector. It seems to imply that one must only add a new element.  So why not call this something like addIfAbsent: ?
> >
> > Here are some suggestions.  Votes?
> > - don't change it; stick with addNewElement:
> > - addIfAbsent:
> > - ifAbsentAdd:
> > - ifMissingAdd:
> >
> > I think I prefer ifAbsentAdd: cuz addIfAbsent: looks too much like a potential spelling error, and conflicts with typical ifAbsent: arguments supplying exception blocks.  But I could go with ifMissingAdd: because it is more distinctive.
>
> Well, we do have
>
>         Collection>>addIfNotPresent:
>
> So why invent a new one?
>
> Because addIfNotPresent: answers its argument and we need one that answers whether the element was absent.  So alas addIfNotPresent: is not a suitable candidate.
>
>
> I think the important thing with #addNewElement: is that it returns *whether* it added a new element
> but then again it breaks the tradition of #add* returning its argument…
>
> a very clear and very strange one that would reveal that a boolean is returned would be #isAbsentAndIfSoAdd: …
>
> wasAbsentAdding: or ifWasAbsentAdding: would be less cumbersome but I like something snappier that people will remember.  ifMissingAdd: looks good because it doesn't conflict with the add*: methods answering their argument, and the ifMissing implies the answer is true if the element wasn't already present.
>
> Stéphane, can you live with ifMissingAdd: ?  Chris?
>
> Best regards
>         -Tobias
>
> > _,,,^..^,,,_
> > best, Eliot
>
> _,,,^..^,,,_
> best, Eliot
 
_,,,^..^,,,_
best, Eliot


Reply | Threaded
Open this post in threaded view
|

Re: The Trunk: Collections-eem.756.mcz

Chris Muller-4
In reply to this post by Eliot Miranda-2
Yes, because of the dual purpose of this method; 1) to ensure presence of an element and, 2) answer whether the element already existed,

to be pedantic, answer if the element didn't already exist

Yes, #addNewElement: answers whether it was a new element.

 
there isn't a clear and concise name which conveys the full nuance.  It's one of those methods which requires an initial look at the comment and/or code until one is familiar with it.

Given that addNewElement: doesn't convey the full nuance that's not a valid criticism of the other alternatives, is it ;-)

Well, I think it sort of does.  The word "add" means its compatible with the Set's existing "add" -- we know it means it'll end up in the Set.  That part of the name is stable.  Then there's that word, "New", which is the part which conveys the special nature of the method.

We could either stick with that, find a new word, or use more than one word.  But I think more words may not add clarity or save one from needing to check the method implementation.
 
I considered those names, too.  I didn't care for addIfAbsent: for the reason you stated.  I don't care for ifAbsentAdd: because it's backward with other "Absent" nomenclature in the API.

And, I didn't go with #ifMissingAdd: because it confuses the return value with the linguistic semantics -- e.g., if it returns true, does that mean it was "absent" or that it was added?  forcing me to look at the method implementation again anyway.

#addNewElement: fits in with the #add... nomenclature part of the API, being like a "command" to the object, to which it may answer, "no" (false), if the element already existed.

It seems the only initial confusion you had was whether it signaled an Exception or not, but I see no other name that avoids the user needing to look at the implementation at least the first time.

No.  The selector implies it might. The selector is bad because this does not add a new element, it ensures an element is present,

It is consistent with Set>>#add:.  I did consider #ensureNewElement:, but that suffers from the implication you mention (that it might signal an Exception) even more.  To me, the #add... prefix puts it in the same neighborhood with the various Set>>#add: methods, and behaves accordingly.
 
and answers if it was added.

Well, it answers if it was, indeed, a "New" element.
  

Therefore, I think #addNewElement: is a good name.

I beg to differ.  I find it misleading :-(  Which is a shame because it's really useful.  How about trying to reach consensus on a less misleading name?

Sure.  I'm not beholden to the name, but I don't know of a better one that's both perfectly clear AND concise.

Best,
  Chris


Reply | Threaded
Open this post in threaded view
|

Re: The Trunk: Collections-eem.756.mcz

Tobias Pape
In reply to this post by Eliot Miranda-2

> On 15.06.2017, at 21:40, Eliot Miranda <[hidden email]> wrote:
>
> Hi Tobias,
>
> On Thu, Jun 15, 2017 at 12:32 PM, Tobias Pape <[hidden email]> wrote:
>
> > On 15.06.2017, at 21:22, Chris Cunningham <[hidden email]> wrote:
> >
> > #testMissingThenAdd:  ?
> >
>
> #isAdding:
> #ingests:
>
> #wasAbsentButNowIsPresent:
>
> (just rambling…)
>
> I like added:.  Its concise, accurate and reads nicely:

Ok, but then maybe #addedNew: ? So as we know more than just an adding operation succeeded… :)

>
> withoutDuplicates
> "Answer a copy of the receiver that preserves order but eliminates any duplicates."
> | seen |
> seen := Set new: self size.
> ^self select: [:each| seen added: each]
>
> messagesDo: aBlock
> "Evaluate aBlock exactly once with all the message selectors sent by me."
>
> | scanner aSet |
> self isQuick ifTrue: [ ^self ].
> scanner := InstructionStream on: self.
> scanner scanFor: [ :x |
> | selector |
> (selector := scanner selectorToSendOrSelf) == scanner ifFalse: [
> ((aSet ifNil: [ aSet := IdentitySet new ]) added: selector) ifTrue: [
> aBlock value: selector ] ].
> false "keep scanning" ]
>
>  
>
> > On Jun 15, 2017 12:19 PM, "Eliot Miranda" <[hidden email]> wrote:
> > Hi Tobias,
> >
> > On Thu, Jun 15, 2017 at 12:06 PM, Tobias Pape <[hidden email]> wrote:
> >
> > > On 15.06.2017, at 20:30, Eliot Miranda <[hidden email]> wrote:
> > >
> > > Hi Levente, Hi Chris,
> > >
> > > On Wed, Jun 14, 2017 at 1:10 PM, Levente Uzonyi <[hidden email]> wrote:
> > > On Wed, 14 Jun 2017, [hidden email] wrote:
> > >
> > > Eliot Miranda uploaded a new version of Collections to project The Trunk:
> > > http://source.squeak.org/trunk/Collections-eem.756.mcz
> > >
> > > ==================== Summary ====================
> > >
> > > Name: Collections-eem.756
> > > Author: eem
> > > Time: 14 June 2017, 11:03:24.917631 am
> > > UUID: 8d7c03bc-1cdb-44c7-9173-10d50c0dae29
> > > Ancestors: Collections-eem.755
> > >
> > > Add SequenceableCollection>>withoutDuplicates for a more elegant fix to MailMessage>>to:
> > >
> > > =============== Diff against Collections-eem.755 ===============
> > >
> > > Item was added:
> > > + ----- Method: SequenceableCollection>>withoutDuplicates (in category 'copying') -----
> > > + withoutDuplicates
> > > +       "Answer a copy of the receiver that preserves order but eliminates any duplicates."
> > > +       | seen |
> > > +       seen := Set new: self size.
> > > +       ^self select: [:each|
> > > +                                 (seen includes: each)
> > > +                                       ifTrue: [false]
> > > +                                       ifFalse: [seen add: each. true]]!
> > >
> > > What a great opportunity to use #addNewElement::
> > >
> > >         ^self select: [ :each | seen addNewElement: each ]
> > >
> > > I love the functionality but I don't like the selector. It seems to imply that one must only add a new element.  So why not call this something like addIfAbsent: ?
> > >
> > > Here are some suggestions.  Votes?
> > > - don't change it; stick with addNewElement:
> > > - addIfAbsent:
> > > - ifAbsentAdd:
> > > - ifMissingAdd:
> > >
> > > I think I prefer ifAbsentAdd: cuz addIfAbsent: looks too much like a potential spelling error, and conflicts with typical ifAbsent: arguments supplying exception blocks.  But I could go with ifMissingAdd: because it is more distinctive.
> >
> > Well, we do have
> >
> >         Collection>>addIfNotPresent:
> >
> > So why invent a new one?
> >
> > Because addIfNotPresent: answers its argument and we need one that answers whether the element was absent.  So alas addIfNotPresent: is not a suitable candidate.
> >
> >
> > I think the important thing with #addNewElement: is that it returns *whether* it added a new element
> > but then again it breaks the tradition of #add* returning its argument…
> >
> > a very clear and very strange one that would reveal that a boolean is returned would be #isAbsentAndIfSoAdd: …
> >
> > wasAbsentAdding: or ifWasAbsentAdding: would be less cumbersome but I like something snappier that people will remember.  ifMissingAdd: looks good because it doesn't conflict with the add*: methods answering their argument, and the ifMissing implies the answer is true if the element wasn't already present.
> >
> > Stéphane, can you live with ifMissingAdd: ?  Chris?
> >
> > Best regards
> >         -Tobias
> >
> > > _,,,^..^,,,_
> > > best, Eliot
> >
> > _,,,^..^,,,_
> > best, Eliot
>  
> _,,,^..^,,,_
> best, Eliot
>


cbc
Reply | Threaded
Open this post in threaded view
|

Re: The Trunk: Collections-eem.756.mcz

cbc


On Thu, Jun 15, 2017 at 1:22 PM, Tobias Pape <[hidden email]> wrote:

> On 15.06.2017, at 21:40, Eliot Miranda <[hidden email]> wrote:
>
> Hi Tobias,
>
> On Thu, Jun 15, 2017 at 12:32 PM, Tobias Pape <[hidden email]> wrote:
>
> > On 15.06.2017, at 21:22, Chris Cunningham <[hidden email]> wrote:
> >
> > #testMissingThenAdd:  ?
> >
>
> #isAdding:
> #ingests:
>
> #wasAbsentButNowIsPresent:
>
> (just rambling…)
>
> I like added:.  Its concise, accurate and reads nicely:

Ok, but then maybe #addedNew: ? So as we know more than just an adding operation succeeded… :)

So it reads:

^self select: [:each| seen addedNew: each] 

I like that how that reads.

#added: wasn't bad - but I could see myself thinking it meant roughly 'was this value already in the collection'?  Which misses the add part and is backwards on the result (true if present, false if added).

-cbc

>
> withoutDuplicates
>       "Answer a copy of the receiver that preserves order but eliminates any duplicates."
>       | seen |
>       seen := Set new: self size.
>       ^self select: [:each| seen added: each]
>
> messagesDo: aBlock
>       "Evaluate aBlock exactly once with all the message selectors sent by me."
>
>       | scanner aSet |
>       self isQuick ifTrue: [ ^self ].
>       scanner := InstructionStream on: self.
>       scanner scanFor: [ :x |
>               | selector |
>               (selector := scanner selectorToSendOrSelf) == scanner ifFalse: [
>                       ((aSet ifNil: [ aSet := IdentitySet new ]) added: selector) ifTrue: [
>                               aBlock value: selector ] ].
>               false "keep scanning" ]
>
>
>
> > On Jun 15, 2017 12:19 PM, "Eliot Miranda" <[hidden email]> wrote:
> > Hi Tobias,
> >
> > On Thu, Jun 15, 2017 at 12:06 PM, Tobias Pape <[hidden email]> wrote:
> >
> > > On 15.06.2017, at 20:30, Eliot Miranda <[hidden email]> wrote:
> > >
> > > Hi Levente, Hi Chris,
> > >
> > > On Wed, Jun 14, 2017 at 1:10 PM, Levente Uzonyi <[hidden email]> wrote:
> > > On Wed, 14 Jun 2017, [hidden email] wrote:
> > >
> > > Eliot Miranda uploaded a new version of Collections to project The Trunk:
> > > http://source.squeak.org/trunk/Collections-eem.756.mcz
> > >
> > > ==================== Summary ====================
> > >
> > > Name: Collections-eem.756
> > > Author: eem
> > > Time: 14 June 2017, 11:03:24.917631 am
> > > UUID: 8d7c03bc-1cdb-44c7-9173-10d50c0dae29
> > > Ancestors: Collections-eem.755
> > >
> > > Add SequenceableCollection>>withoutDuplicates for a more elegant fix to MailMessage>>to:
> > >
> > > =============== Diff against Collections-eem.755 ===============
> > >
> > > Item was added:
> > > + ----- Method: SequenceableCollection>>withoutDuplicates (in category 'copying') -----
> > > + withoutDuplicates
> > > +       "Answer a copy of the receiver that preserves order but eliminates any duplicates."
> > > +       | seen |
> > > +       seen := Set new: self size.
> > > +       ^self select: [:each|
> > > +                                 (seen includes: each)
> > > +                                       ifTrue: [false]
> > > +                                       ifFalse: [seen add: each. true]]!
> > >
> > > What a great opportunity to use #addNewElement::
> > >
> > >         ^self select: [ :each | seen addNewElement: each ]
> > >
> > > I love the functionality but I don't like the selector. It seems to imply that one must only add a new element.  So why not call this something like addIfAbsent: ?
> > >
> > > Here are some suggestions.  Votes?
> > > - don't change it; stick with addNewElement:
> > > - addIfAbsent:
> > > - ifAbsentAdd:
> > > - ifMissingAdd:
> > >
> > > I think I prefer ifAbsentAdd: cuz addIfAbsent: looks too much like a potential spelling error, and conflicts with typical ifAbsent: arguments supplying exception blocks.  But I could go with ifMissingAdd: because it is more distinctive.
> >
> > Well, we do have
> >
> >         Collection>>addIfNotPresent:
> >
> > So why invent a new one?
> >
> > Because addIfNotPresent: answers its argument and we need one that answers whether the element was absent.  So alas addIfNotPresent: is not a suitable candidate.
> >
> >
> > I think the important thing with #addNewElement: is that it returns *whether* it added a new element
> > but then again it breaks the tradition of #add* returning its argument…
> >
> > a very clear and very strange one that would reveal that a boolean is returned would be #isAbsentAndIfSoAdd: …
> >
> > wasAbsentAdding: or ifWasAbsentAdding: would be less cumbersome but I like something snappier that people will remember.  ifMissingAdd: looks good because it doesn't conflict with the add*: methods answering their argument, and the ifMissing implies the answer is true if the element wasn't already present.
> >
> > Stéphane, can you live with ifMissingAdd: ?  Chris?
> >
> > Best regards
> >         -Tobias
> >
> > > _,,,^..^,,,_
> > > best, Eliot
> >
> > _,,,^..^,,,_
> > best, Eliot
>
> _,,,^..^,,,_
> best, Eliot
>





Reply | Threaded
Open this post in threaded view
|

Re: The Trunk: Collections-eem.756.mcz

Chris Muller-3
In reply to this post by Tobias Pape
I like the conciseness, but #added: seems like a non-mutative
"testing" message like #includes:, instead of a directive like #add...


On Thu, Jun 15, 2017 at 3:22 PM, Tobias Pape <[hidden email]> wrote:

>
>> On 15.06.2017, at 21:40, Eliot Miranda <[hidden email]> wrote:
>>
>> Hi Tobias,
>>
>> On Thu, Jun 15, 2017 at 12:32 PM, Tobias Pape <[hidden email]> wrote:
>>
>> > On 15.06.2017, at 21:22, Chris Cunningham <[hidden email]> wrote:
>> >
>> > #testMissingThenAdd:  ?
>> >
>>
>> #isAdding:
>> #ingests:
>>
>> #wasAbsentButNowIsPresent:
>>
>> (just rambling…)
>>
>> I like added:.  Its concise, accurate and reads nicely:
>
> Ok, but then maybe #addedNew: ? So as we know more than just an adding operation succeeded… :)
>
>>
>> withoutDuplicates
>>       "Answer a copy of the receiver that preserves order but eliminates any duplicates."
>>       | seen |
>>       seen := Set new: self size.
>>       ^self select: [:each| seen added: each]
>>
>> messagesDo: aBlock
>>       "Evaluate aBlock exactly once with all the message selectors sent by me."
>>
>>       | scanner aSet |
>>       self isQuick ifTrue: [ ^self ].
>>       scanner := InstructionStream on: self.
>>       scanner scanFor: [ :x |
>>               | selector |
>>               (selector := scanner selectorToSendOrSelf) == scanner ifFalse: [
>>                       ((aSet ifNil: [ aSet := IdentitySet new ]) added: selector) ifTrue: [
>>                               aBlock value: selector ] ].
>>               false "keep scanning" ]
>>
>>
>>
>> > On Jun 15, 2017 12:19 PM, "Eliot Miranda" <[hidden email]> wrote:
>> > Hi Tobias,
>> >
>> > On Thu, Jun 15, 2017 at 12:06 PM, Tobias Pape <[hidden email]> wrote:
>> >
>> > > On 15.06.2017, at 20:30, Eliot Miranda <[hidden email]> wrote:
>> > >
>> > > Hi Levente, Hi Chris,
>> > >
>> > > On Wed, Jun 14, 2017 at 1:10 PM, Levente Uzonyi <[hidden email]> wrote:
>> > > On Wed, 14 Jun 2017, [hidden email] wrote:
>> > >
>> > > Eliot Miranda uploaded a new version of Collections to project The Trunk:
>> > > http://source.squeak.org/trunk/Collections-eem.756.mcz
>> > >
>> > > ==================== Summary ====================
>> > >
>> > > Name: Collections-eem.756
>> > > Author: eem
>> > > Time: 14 June 2017, 11:03:24.917631 am
>> > > UUID: 8d7c03bc-1cdb-44c7-9173-10d50c0dae29
>> > > Ancestors: Collections-eem.755
>> > >
>> > > Add SequenceableCollection>>withoutDuplicates for a more elegant fix to MailMessage>>to:
>> > >
>> > > =============== Diff against Collections-eem.755 ===============
>> > >
>> > > Item was added:
>> > > + ----- Method: SequenceableCollection>>withoutDuplicates (in category 'copying') -----
>> > > + withoutDuplicates
>> > > +       "Answer a copy of the receiver that preserves order but eliminates any duplicates."
>> > > +       | seen |
>> > > +       seen := Set new: self size.
>> > > +       ^self select: [:each|
>> > > +                                 (seen includes: each)
>> > > +                                       ifTrue: [false]
>> > > +                                       ifFalse: [seen add: each. true]]!
>> > >
>> > > What a great opportunity to use #addNewElement::
>> > >
>> > >         ^self select: [ :each | seen addNewElement: each ]
>> > >
>> > > I love the functionality but I don't like the selector. It seems to imply that one must only add a new element.  So why not call this something like addIfAbsent: ?
>> > >
>> > > Here are some suggestions.  Votes?
>> > > - don't change it; stick with addNewElement:
>> > > - addIfAbsent:
>> > > - ifAbsentAdd:
>> > > - ifMissingAdd:
>> > >
>> > > I think I prefer ifAbsentAdd: cuz addIfAbsent: looks too much like a potential spelling error, and conflicts with typical ifAbsent: arguments supplying exception blocks.  But I could go with ifMissingAdd: because it is more distinctive.
>> >
>> > Well, we do have
>> >
>> >         Collection>>addIfNotPresent:
>> >
>> > So why invent a new one?
>> >
>> > Because addIfNotPresent: answers its argument and we need one that answers whether the element was absent.  So alas addIfNotPresent: is not a suitable candidate.
>> >
>> >
>> > I think the important thing with #addNewElement: is that it returns *whether* it added a new element
>> > but then again it breaks the tradition of #add* returning its argument…
>> >
>> > a very clear and very strange one that would reveal that a boolean is returned would be #isAbsentAndIfSoAdd: …
>> >
>> > wasAbsentAdding: or ifWasAbsentAdding: would be less cumbersome but I like something snappier that people will remember.  ifMissingAdd: looks good because it doesn't conflict with the add*: methods answering their argument, and the ifMissing implies the answer is true if the element wasn't already present.
>> >
>> > Stéphane, can you live with ifMissingAdd: ?  Chris?
>> >
>> > Best regards
>> >         -Tobias
>> >
>> > > _,,,^..^,,,_
>> > > best, Eliot
>> >
>> > _,,,^..^,,,_
>> > best, Eliot
>>
>> _,,,^..^,,,_
>> best, Eliot
>>
>
>

Reply | Threaded
Open this post in threaded view
|

Re: The Trunk: Collections-eem.756.mcz

Chris Muller-3
In reply to this post by cbc
^ self select: [ : each | seen addNew: each ]

?


On Thu, Jun 15, 2017 at 3:50 PM, Chris Cunningham
<[hidden email]> wrote:

>
>
> On Thu, Jun 15, 2017 at 1:22 PM, Tobias Pape <[hidden email]> wrote:
>>
>>
>> > On 15.06.2017, at 21:40, Eliot Miranda <[hidden email]> wrote:
>> >
>> > Hi Tobias,
>> >
>> > On Thu, Jun 15, 2017 at 12:32 PM, Tobias Pape <[hidden email]> wrote:
>> >
>> > > On 15.06.2017, at 21:22, Chris Cunningham <[hidden email]>
>> > > wrote:
>> > >
>> > > #testMissingThenAdd:  ?
>> > >
>> >
>> > #isAdding:
>> > #ingests:
>> >
>> > #wasAbsentButNowIsPresent:
>> >
>> > (just rambling…)
>> >
>> > I like added:.  Its concise, accurate and reads nicely:
>>
>>
>> Ok, but then maybe #addedNew: ? So as we know more than just an adding
>> operation succeeded… :)
>
>
> So it reads:
>
> ^self select: [:each| seen addedNew: each]
>
> I like that how that reads.
>
> #added: wasn't bad - but I could see myself thinking it meant roughly 'was
> this value already in the collection'?  Which misses the add part and is
> backwards on the result (true if present, false if added).
>
> -cbc
>>
>>
>> >
>> > withoutDuplicates
>> >       "Answer a copy of the receiver that preserves order but eliminates
>> > any duplicates."
>> >       | seen |
>> >       seen := Set new: self size.
>> >       ^self select: [:each| seen added: each]
>> >
>> > messagesDo: aBlock
>> >       "Evaluate aBlock exactly once with all the message selectors sent
>> > by me."
>> >
>> >       | scanner aSet |
>> >       self isQuick ifTrue: [ ^self ].
>> >       scanner := InstructionStream on: self.
>> >       scanner scanFor: [ :x |
>> >               | selector |
>> >               (selector := scanner selectorToSendOrSelf) == scanner
>> > ifFalse: [
>> >                       ((aSet ifNil: [ aSet := IdentitySet new ]) added:
>> > selector) ifTrue: [
>> >                               aBlock value: selector ] ].
>> >               false "keep scanning" ]
>> >
>> >
>> >
>> > > On Jun 15, 2017 12:19 PM, "Eliot Miranda" <[hidden email]>
>> > > wrote:
>> > > Hi Tobias,
>> > >
>> > > On Thu, Jun 15, 2017 at 12:06 PM, Tobias Pape <[hidden email]>
>> > > wrote:
>> > >
>> > > > On 15.06.2017, at 20:30, Eliot Miranda <[hidden email]>
>> > > > wrote:
>> > > >
>> > > > Hi Levente, Hi Chris,
>> > > >
>> > > > On Wed, Jun 14, 2017 at 1:10 PM, Levente Uzonyi
>> > > > <[hidden email]> wrote:
>> > > > On Wed, 14 Jun 2017, [hidden email] wrote:
>> > > >
>> > > > Eliot Miranda uploaded a new version of Collections to project The
>> > > > Trunk:
>> > > > http://source.squeak.org/trunk/Collections-eem.756.mcz
>> > > >
>> > > > ==================== Summary ====================
>> > > >
>> > > > Name: Collections-eem.756
>> > > > Author: eem
>> > > > Time: 14 June 2017, 11:03:24.917631 am
>> > > > UUID: 8d7c03bc-1cdb-44c7-9173-10d50c0dae29
>> > > > Ancestors: Collections-eem.755
>> > > >
>> > > > Add SequenceableCollection>>withoutDuplicates for a more elegant fix
>> > > > to MailMessage>>to:
>> > > >
>> > > > =============== Diff against Collections-eem.755 ===============
>> > > >
>> > > > Item was added:
>> > > > + ----- Method: SequenceableCollection>>withoutDuplicates (in
>> > > > category 'copying') -----
>> > > > + withoutDuplicates
>> > > > +       "Answer a copy of the receiver that preserves order but
>> > > > eliminates any duplicates."
>> > > > +       | seen |
>> > > > +       seen := Set new: self size.
>> > > > +       ^self select: [:each|
>> > > > +                                 (seen includes: each)
>> > > > +                                       ifTrue: [false]
>> > > > +                                       ifFalse: [seen add: each.
>> > > > true]]!
>> > > >
>> > > > What a great opportunity to use #addNewElement::
>> > > >
>> > > >         ^self select: [ :each | seen addNewElement: each ]
>> > > >
>> > > > I love the functionality but I don't like the selector. It seems to
>> > > > imply that one must only add a new element.  So why not call this something
>> > > > like addIfAbsent: ?
>> > > >
>> > > > Here are some suggestions.  Votes?
>> > > > - don't change it; stick with addNewElement:
>> > > > - addIfAbsent:
>> > > > - ifAbsentAdd:
>> > > > - ifMissingAdd:
>> > > >
>> > > > I think I prefer ifAbsentAdd: cuz addIfAbsent: looks too much like a
>> > > > potential spelling error, and conflicts with typical ifAbsent: arguments
>> > > > supplying exception blocks.  But I could go with ifMissingAdd: because it is
>> > > > more distinctive.
>> > >
>> > > Well, we do have
>> > >
>> > >         Collection>>addIfNotPresent:
>> > >
>> > > So why invent a new one?
>> > >
>> > > Because addIfNotPresent: answers its argument and we need one that
>> > > answers whether the element was absent.  So alas addIfNotPresent: is not a
>> > > suitable candidate.
>> > >
>> > >
>> > > I think the important thing with #addNewElement: is that it returns
>> > > *whether* it added a new element
>> > > but then again it breaks the tradition of #add* returning its
>> > > argument…
>> > >
>> > > a very clear and very strange one that would reveal that a boolean is
>> > > returned would be #isAbsentAndIfSoAdd: …
>> > >
>> > > wasAbsentAdding: or ifWasAbsentAdding: would be less cumbersome but I
>> > > like something snappier that people will remember.  ifMissingAdd: looks good
>> > > because it doesn't conflict with the add*: methods answering their argument,
>> > > and the ifMissing implies the answer is true if the element wasn't already
>> > > present.
>> > >
>> > > Stéphane, can you live with ifMissingAdd: ?  Chris?
>> > >
>> > > Best regards
>> > >         -Tobias
>> > >
>> > > > _,,,^..^,,,_
>> > > > best, Eliot
>> > >
>> > > _,,,^..^,,,_
>> > > best, Eliot
>> >
>> > _,,,^..^,,,_
>> > best, Eliot
>> >
>>
>>
>
>
>
>

Reply | Threaded
Open this post in threaded view
|

Re: The Trunk: Collections-eem.756.mcz

Nicolas Cellier
maybe addOnce: would be more expressive than addNew:?


2017-06-15 22:52 GMT+02:00 Chris Muller <[hidden email]>:
^ self select: [ : each | seen addNew: each ]

?


On Thu, Jun 15, 2017 at 3:50 PM, Chris Cunningham
<[hidden email]> wrote:
>
>
> On Thu, Jun 15, 2017 at 1:22 PM, Tobias Pape <[hidden email]> wrote:
>>
>>
>> > On 15.06.2017, at 21:40, Eliot Miranda <[hidden email]> wrote:
>> >
>> > Hi Tobias,
>> >
>> > On Thu, Jun 15, 2017 at 12:32 PM, Tobias Pape <[hidden email]> wrote:
>> >
>> > > On 15.06.2017, at 21:22, Chris Cunningham <[hidden email]>
>> > > wrote:
>> > >
>> > > #testMissingThenAdd:  ?
>> > >
>> >
>> > #isAdding:
>> > #ingests:
>> >
>> > #wasAbsentButNowIsPresent:
>> >
>> > (just rambling…)
>> >
>> > I like added:.  Its concise, accurate and reads nicely:
>>
>>
>> Ok, but then maybe #addedNew: ? So as we know more than just an adding
>> operation succeeded… :)
>
>
> So it reads:
>
> ^self select: [:each| seen addedNew: each]
>
> I like that how that reads.
>
> #added: wasn't bad - but I could see myself thinking it meant roughly 'was
> this value already in the collection'?  Which misses the add part and is
> backwards on the result (true if present, false if added).
>
> -cbc
>>
>>
>> >
>> > withoutDuplicates
>> >       "Answer a copy of the receiver that preserves order but eliminates
>> > any duplicates."
>> >       | seen |
>> >       seen := Set new: self size.
>> >       ^self select: [:each| seen added: each]
>> >
>> > messagesDo: aBlock
>> >       "Evaluate aBlock exactly once with all the message selectors sent
>> > by me."
>> >
>> >       | scanner aSet |
>> >       self isQuick ifTrue: [ ^self ].
>> >       scanner := InstructionStream on: self.
>> >       scanner scanFor: [ :x |
>> >               | selector |
>> >               (selector := scanner selectorToSendOrSelf) == scanner
>> > ifFalse: [
>> >                       ((aSet ifNil: [ aSet := IdentitySet new ]) added:
>> > selector) ifTrue: [
>> >                               aBlock value: selector ] ].
>> >               false "keep scanning" ]
>> >
>> >
>> >
>> > > On Jun 15, 2017 12:19 PM, "Eliot Miranda" <[hidden email]>
>> > > wrote:
>> > > Hi Tobias,
>> > >
>> > > On Thu, Jun 15, 2017 at 12:06 PM, Tobias Pape <[hidden email]>
>> > > wrote:
>> > >
>> > > > On 15.06.2017, at 20:30, Eliot Miranda <[hidden email]>
>> > > > wrote:
>> > > >
>> > > > Hi Levente, Hi Chris,
>> > > >
>> > > > On Wed, Jun 14, 2017 at 1:10 PM, Levente Uzonyi
>> > > > <[hidden email]> wrote:
>> > > > On Wed, 14 Jun 2017, [hidden email] wrote:
>> > > >
>> > > > Eliot Miranda uploaded a new version of Collections to project The
>> > > > Trunk:
>> > > > http://source.squeak.org/trunk/Collections-eem.756.mcz
>> > > >
>> > > > ==================== Summary ====================
>> > > >
>> > > > Name: Collections-eem.756
>> > > > Author: eem
>> > > > Time: 14 June 2017, 11:03:24.917631 am
>> > > > UUID: 8d7c03bc-1cdb-44c7-9173-10d50c0dae29
>> > > > Ancestors: Collections-eem.755
>> > > >
>> > > > Add SequenceableCollection>>withoutDuplicates for a more elegant fix
>> > > > to MailMessage>>to:
>> > > >
>> > > > =============== Diff against Collections-eem.755 ===============
>> > > >
>> > > > Item was added:
>> > > > + ----- Method: SequenceableCollection>>withoutDuplicates (in
>> > > > category 'copying') -----
>> > > > + withoutDuplicates
>> > > > +       "Answer a copy of the receiver that preserves order but
>> > > > eliminates any duplicates."
>> > > > +       | seen |
>> > > > +       seen := Set new: self size.
>> > > > +       ^self select: [:each|
>> > > > +                                 (seen includes: each)
>> > > > +                                       ifTrue: [false]
>> > > > +                                       ifFalse: [seen add: each.
>> > > > true]]!
>> > > >
>> > > > What a great opportunity to use #addNewElement::
>> > > >
>> > > >         ^self select: [ :each | seen addNewElement: each ]
>> > > >
>> > > > I love the functionality but I don't like the selector. It seems to
>> > > > imply that one must only add a new element.  So why not call this something
>> > > > like addIfAbsent: ?
>> > > >
>> > > > Here are some suggestions.  Votes?
>> > > > - don't change it; stick with addNewElement:
>> > > > - addIfAbsent:
>> > > > - ifAbsentAdd:
>> > > > - ifMissingAdd:
>> > > >
>> > > > I think I prefer ifAbsentAdd: cuz addIfAbsent: looks too much like a
>> > > > potential spelling error, and conflicts with typical ifAbsent: arguments
>> > > > supplying exception blocks.  But I could go with ifMissingAdd: because it is
>> > > > more distinctive.
>> > >
>> > > Well, we do have
>> > >
>> > >         Collection>>addIfNotPresent:
>> > >
>> > > So why invent a new one?
>> > >
>> > > Because addIfNotPresent: answers its argument and we need one that
>> > > answers whether the element was absent.  So alas addIfNotPresent: is not a
>> > > suitable candidate.
>> > >
>> > >
>> > > I think the important thing with #addNewElement: is that it returns
>> > > *whether* it added a new element
>> > > but then again it breaks the tradition of #add* returning its
>> > > argument…
>> > >
>> > > a very clear and very strange one that would reveal that a boolean is
>> > > returned would be #isAbsentAndIfSoAdd: …
>> > >
>> > > wasAbsentAdding: or ifWasAbsentAdding: would be less cumbersome but I
>> > > like something snappier that people will remember.  ifMissingAdd: looks good
>> > > because it doesn't conflict with the add*: methods answering their argument,
>> > > and the ifMissing implies the answer is true if the element wasn't already
>> > > present.
>> > >
>> > > Stéphane, can you live with ifMissingAdd: ?  Chris?
>> > >
>> > > Best regards
>> > >         -Tobias
>> > >
>> > > > _,,,^..^,,,_
>> > > > best, Eliot
>> > >
>> > > _,,,^..^,,,_
>> > > best, Eliot
>> >
>> > _,,,^..^,,,_
>> > best, Eliot
>> >
>>
>>
>
>
>
>




Reply | Threaded
Open this post in threaded view
|

Re: The Trunk: Collections-eem.756.mcz

Tobias Pape
In reply to this post by Chris Muller-3

> On 15.06.2017, at 22:52, Chris Muller <[hidden email]> wrote:
>
> ^ self select: [ : each | seen addNew: each ]

I would expect that to return each not true/false…

>
> ?
>
>
> On Thu, Jun 15, 2017 at 3:50 PM, Chris Cunningham
> <[hidden email]> wrote:
>>
>>
>> On Thu, Jun 15, 2017 at 1:22 PM, Tobias Pape <[hidden email]> wrote:
>>>
>>>
>>>> On 15.06.2017, at 21:40, Eliot Miranda <[hidden email]> wrote:
>>>>
>>>> Hi Tobias,
>>>>
>>>> On Thu, Jun 15, 2017 at 12:32 PM, Tobias Pape <[hidden email]> wrote:
>>>>
>>>>> On 15.06.2017, at 21:22, Chris Cunningham <[hidden email]>
>>>>> wrote:
>>>>>
>>>>> #testMissingThenAdd:  ?
>>>>>
>>>>
>>>> #isAdding:
>>>> #ingests:
>>>>
>>>> #wasAbsentButNowIsPresent:
>>>>
>>>> (just rambling…)
>>>>
>>>> I like added:.  Its concise, accurate and reads nicely:
>>>
>>>
>>> Ok, but then maybe #addedNew: ? So as we know more than just an adding
>>> operation succeeded… :)
>>
>>
>> So it reads:
>>
>> ^self select: [:each| seen addedNew: each]
>>
>> I like that how that reads.
>>
>> #added: wasn't bad - but I could see myself thinking it meant roughly 'was
>> this value already in the collection'?  Which misses the add part and is
>> backwards on the result (true if present, false if added).
>>
>> -cbc
>>>
>>>
>>>>
>>>> withoutDuplicates
>>>>      "Answer a copy of the receiver that preserves order but eliminates
>>>> any duplicates."
>>>>      | seen |
>>>>      seen := Set new: self size.
>>>>      ^self select: [:each| seen added: each]
>>>>
>>>> messagesDo: aBlock
>>>>      "Evaluate aBlock exactly once with all the message selectors sent
>>>> by me."
>>>>
>>>>      | scanner aSet |
>>>>      self isQuick ifTrue: [ ^self ].
>>>>      scanner := InstructionStream on: self.
>>>>      scanner scanFor: [ :x |
>>>>              | selector |
>>>>              (selector := scanner selectorToSendOrSelf) == scanner
>>>> ifFalse: [
>>>>                      ((aSet ifNil: [ aSet := IdentitySet new ]) added:
>>>> selector) ifTrue: [
>>>>                              aBlock value: selector ] ].
>>>>              false "keep scanning" ]
>>>>
>>>>
>>>>
>>>>> On Jun 15, 2017 12:19 PM, "Eliot Miranda" <[hidden email]>
>>>>> wrote:
>>>>> Hi Tobias,
>>>>>
>>>>> On Thu, Jun 15, 2017 at 12:06 PM, Tobias Pape <[hidden email]>
>>>>> wrote:
>>>>>
>>>>>> On 15.06.2017, at 20:30, Eliot Miranda <[hidden email]>
>>>>>> wrote:
>>>>>>
>>>>>> Hi Levente, Hi Chris,
>>>>>>
>>>>>> On Wed, Jun 14, 2017 at 1:10 PM, Levente Uzonyi
>>>>>> <[hidden email]> wrote:
>>>>>> On Wed, 14 Jun 2017, [hidden email] wrote:
>>>>>>
>>>>>> Eliot Miranda uploaded a new version of Collections to project The
>>>>>> Trunk:
>>>>>> http://source.squeak.org/trunk/Collections-eem.756.mcz
>>>>>>
>>>>>> ==================== Summary ====================
>>>>>>
>>>>>> Name: Collections-eem.756
>>>>>> Author: eem
>>>>>> Time: 14 June 2017, 11:03:24.917631 am
>>>>>> UUID: 8d7c03bc-1cdb-44c7-9173-10d50c0dae29
>>>>>> Ancestors: Collections-eem.755
>>>>>>
>>>>>> Add SequenceableCollection>>withoutDuplicates for a more elegant fix
>>>>>> to MailMessage>>to:
>>>>>>
>>>>>> =============== Diff against Collections-eem.755 ===============
>>>>>>
>>>>>> Item was added:
>>>>>> + ----- Method: SequenceableCollection>>withoutDuplicates (in
>>>>>> category 'copying') -----
>>>>>> + withoutDuplicates
>>>>>> +       "Answer a copy of the receiver that preserves order but
>>>>>> eliminates any duplicates."
>>>>>> +       | seen |
>>>>>> +       seen := Set new: self size.
>>>>>> +       ^self select: [:each|
>>>>>> +                                 (seen includes: each)
>>>>>> +                                       ifTrue: [false]
>>>>>> +                                       ifFalse: [seen add: each.
>>>>>> true]]!
>>>>>>
>>>>>> What a great opportunity to use #addNewElement::
>>>>>>
>>>>>>        ^self select: [ :each | seen addNewElement: each ]
>>>>>>
>>>>>> I love the functionality but I don't like the selector. It seems to
>>>>>> imply that one must only add a new element.  So why not call this something
>>>>>> like addIfAbsent: ?
>>>>>>
>>>>>> Here are some suggestions.  Votes?
>>>>>> - don't change it; stick with addNewElement:
>>>>>> - addIfAbsent:
>>>>>> - ifAbsentAdd:
>>>>>> - ifMissingAdd:
>>>>>>
>>>>>> I think I prefer ifAbsentAdd: cuz addIfAbsent: looks too much like a
>>>>>> potential spelling error, and conflicts with typical ifAbsent: arguments
>>>>>> supplying exception blocks.  But I could go with ifMissingAdd: because it is
>>>>>> more distinctive.
>>>>>
>>>>> Well, we do have
>>>>>
>>>>>        Collection>>addIfNotPresent:
>>>>>
>>>>> So why invent a new one?
>>>>>
>>>>> Because addIfNotPresent: answers its argument and we need one that
>>>>> answers whether the element was absent.  So alas addIfNotPresent: is not a
>>>>> suitable candidate.
>>>>>
>>>>>
>>>>> I think the important thing with #addNewElement: is that it returns
>>>>> *whether* it added a new element
>>>>> but then again it breaks the tradition of #add* returning its
>>>>> argument…
>>>>>
>>>>> a very clear and very strange one that would reveal that a boolean is
>>>>> returned would be #isAbsentAndIfSoAdd: …
>>>>>
>>>>> wasAbsentAdding: or ifWasAbsentAdding: would be less cumbersome but I
>>>>> like something snappier that people will remember.  ifMissingAdd: looks good
>>>>> because it doesn't conflict with the add*: methods answering their argument,
>>>>> and the ifMissing implies the answer is true if the element wasn't already
>>>>> present.
>>>>>
>>>>> Stéphane, can you live with ifMissingAdd: ?  Chris?
>>>>>
>>>>> Best regards
>>>>>        -Tobias
>>>>>
>>>>>> _,,,^..^,,,_
>>>>>> best, Eliot
>>>>>
>>>>> _,,,^..^,,,_
>>>>> best, Eliot
>>>>
>>>> _,,,^..^,,,_
>>>> best, Eliot
>>>>
>>>
>>>
>>
>>
>>
>>
>


cbc
Reply | Threaded
Open this post in threaded view
|

Re: The Trunk: Collections-eem.756.mcz

cbc
In reply to this post by Nicolas Cellier


On Thu, Jun 15, 2017 at 1:59 PM, Nicolas Cellier <[hidden email]> wrote:
maybe addOnce: would be more expressive than addNew:?
addedOnce: to remove the expectation that it would return the value (or self)?
It is mixing the test result and the command together in one selector - where else do we do that in core Squeak?



2017-06-15 22:52 GMT+02:00 Chris Muller <[hidden email]>:
^ self select: [ : each | seen addNew: each ]

?


On Thu, Jun 15, 2017 at 3:50 PM, Chris Cunningham
<[hidden email]> wrote:
>
>
> On Thu, Jun 15, 2017 at 1:22 PM, Tobias Pape <[hidden email]> wrote:
>>
>>
>> > On 15.06.2017, at 21:40, Eliot Miranda <[hidden email]> wrote:
>> >
>> > Hi Tobias,
>> >
>> > On Thu, Jun 15, 2017 at 12:32 PM, Tobias Pape <[hidden email]> wrote:
>> >
>> > > On 15.06.2017, at 21:22, Chris Cunningham <[hidden email]>
>> > > wrote:
>> > >
>> > > #testMissingThenAdd:  ?
>> > >
>> >
>> > #isAdding:
>> > #ingests:
>> >
>> > #wasAbsentButNowIsPresent:
>> >
>> > (just rambling…)
>> >
>> > I like added:.  Its concise, accurate and reads nicely:
>>
>>
>> Ok, but then maybe #addedNew: ? So as we know more than just an adding
>> operation succeeded… :)
>
>
> So it reads:
>
> ^self select: [:each| seen addedNew: each]
>
> I like that how that reads.
>
> #added: wasn't bad - but I could see myself thinking it meant roughly 'was
> this value already in the collection'?  Which misses the add part and is
> backwards on the result (true if present, false if added).
>
> -cbc
>>
>>
>> >
>> > withoutDuplicates
>> >       "Answer a copy of the receiver that preserves order but eliminates
>> > any duplicates."
>> >       | seen |
>> >       seen := Set new: self size.
>> >       ^self select: [:each| seen added: each]
>> >
>> > messagesDo: aBlock
>> >       "Evaluate aBlock exactly once with all the message selectors sent
>> > by me."
>> >
>> >       | scanner aSet |
>> >       self isQuick ifTrue: [ ^self ].
>> >       scanner := InstructionStream on: self.
>> >       scanner scanFor: [ :x |
>> >               | selector |
>> >               (selector := scanner selectorToSendOrSelf) == scanner
>> > ifFalse: [
>> >                       ((aSet ifNil: [ aSet := IdentitySet new ]) added:
>> > selector) ifTrue: [
>> >                               aBlock value: selector ] ].
>> >               false "keep scanning" ]
>> >
>> >
>> >
>> > > On Jun 15, 2017 12:19 PM, "Eliot Miranda" <[hidden email]>
>> > > wrote:
>> > > Hi Tobias,
>> > >
>> > > On Thu, Jun 15, 2017 at 12:06 PM, Tobias Pape <[hidden email]>
>> > > wrote:
>> > >
>> > > > On 15.06.2017, at 20:30, Eliot Miranda <[hidden email]>
>> > > > wrote:
>> > > >
>> > > > Hi Levente, Hi Chris,
>> > > >
>> > > > On Wed, Jun 14, 2017 at 1:10 PM, Levente Uzonyi
>> > > > <[hidden email]> wrote:
>> > > > On Wed, 14 Jun 2017, [hidden email] wrote:
>> > > >
>> > > > Eliot Miranda uploaded a new version of Collections to project The
>> > > > Trunk:
>> > > > http://source.squeak.org/trunk/Collections-eem.756.mcz
>> > > >
>> > > > ==================== Summary ====================
>> > > >
>> > > > Name: Collections-eem.756
>> > > > Author: eem
>> > > > Time: 14 June 2017, 11:03:24.917631 am
>> > > > UUID: 8d7c03bc-1cdb-44c7-9173-10d50c0dae29
>> > > > Ancestors: Collections-eem.755
>> > > >
>> > > > Add SequenceableCollection>>withoutDuplicates for a more elegant fix
>> > > > to MailMessage>>to:
>> > > >
>> > > > =============== Diff against Collections-eem.755 ===============
>> > > >
>> > > > Item was added:
>> > > > + ----- Method: SequenceableCollection>>withoutDuplicates (in
>> > > > category 'copying') -----
>> > > > + withoutDuplicates
>> > > > +       "Answer a copy of the receiver that preserves order but
>> > > > eliminates any duplicates."
>> > > > +       | seen |
>> > > > +       seen := Set new: self size.
>> > > > +       ^self select: [:each|
>> > > > +                                 (seen includes: each)
>> > > > +                                       ifTrue: [false]
>> > > > +                                       ifFalse: [seen add: each.
>> > > > true]]!
>> > > >
>> > > > What a great opportunity to use #addNewElement::
>> > > >
>> > > >         ^self select: [ :each | seen addNewElement: each ]
>> > > >
>> > > > I love the functionality but I don't like the selector. It seems to
>> > > > imply that one must only add a new element.  So why not call this something
>> > > > like addIfAbsent: ?
>> > > >
>> > > > Here are some suggestions.  Votes?
>> > > > - don't change it; stick with addNewElement:
>> > > > - addIfAbsent:
>> > > > - ifAbsentAdd:
>> > > > - ifMissingAdd:
>> > > >
>> > > > I think I prefer ifAbsentAdd: cuz addIfAbsent: looks too much like a
>> > > > potential spelling error, and conflicts with typical ifAbsent: arguments
>> > > > supplying exception blocks.  But I could go with ifMissingAdd: because it is
>> > > > more distinctive.
>> > >
>> > > Well, we do have
>> > >
>> > >         Collection>>addIfNotPresent:
>> > >
>> > > So why invent a new one?
>> > >
>> > > Because addIfNotPresent: answers its argument and we need one that
>> > > answers whether the element was absent.  So alas addIfNotPresent: is not a
>> > > suitable candidate.
>> > >
>> > >
>> > > I think the important thing with #addNewElement: is that it returns
>> > > *whether* it added a new element
>> > > but then again it breaks the tradition of #add* returning its
>> > > argument…
>> > >
>> > > a very clear and very strange one that would reveal that a boolean is
>> > > returned would be #isAbsentAndIfSoAdd: …
>> > >
>> > > wasAbsentAdding: or ifWasAbsentAdding: would be less cumbersome but I
>> > > like something snappier that people will remember.  ifMissingAdd: looks good
>> > > because it doesn't conflict with the add*: methods answering their argument,
>> > > and the ifMissing implies the answer is true if the element wasn't already
>> > > present.
>> > >
>> > > Stéphane, can you live with ifMissingAdd: ?  Chris?
>> > >
>> > > Best regards
>> > >         -Tobias
>> > >
>> > > > _,,,^..^,,,_
>> > > > best, Eliot
>> > >
>> > > _,,,^..^,,,_
>> > > best, Eliot
>> >
>> > _,,,^..^,,,_
>> > best, Eliot
>> >
>>
>>
>
>
>
>








Reply | Threaded
Open this post in threaded view
|

Re: The Trunk: Collections-eem.756.mcz

Chris Muller-3
In reply to this post by Tobias Pape
You may also expect it to not to be identical to #add:.


On Thu, Jun 15, 2017 at 4:03 PM, Tobias Pape <[hidden email]> wrote:

>
>> On 15.06.2017, at 22:52, Chris Muller <[hidden email]> wrote:
>>
>> ^ self select: [ : each | seen addNew: each ]
>
> I would expect that to return each not true/false…
>>
>> ?
>>
>>
>> On Thu, Jun 15, 2017 at 3:50 PM, Chris Cunningham
>> <[hidden email]> wrote:
>>>
>>>
>>> On Thu, Jun 15, 2017 at 1:22 PM, Tobias Pape <[hidden email]> wrote:
>>>>
>>>>
>>>>> On 15.06.2017, at 21:40, Eliot Miranda <[hidden email]> wrote:
>>>>>
>>>>> Hi Tobias,
>>>>>
>>>>> On Thu, Jun 15, 2017 at 12:32 PM, Tobias Pape <[hidden email]> wrote:
>>>>>
>>>>>> On 15.06.2017, at 21:22, Chris Cunningham <[hidden email]>
>>>>>> wrote:
>>>>>>
>>>>>> #testMissingThenAdd:  ?
>>>>>>
>>>>>
>>>>> #isAdding:
>>>>> #ingests:
>>>>>
>>>>> #wasAbsentButNowIsPresent:
>>>>>
>>>>> (just rambling…)
>>>>>
>>>>> I like added:.  Its concise, accurate and reads nicely:
>>>>
>>>>
>>>> Ok, but then maybe #addedNew: ? So as we know more than just an adding
>>>> operation succeeded… :)
>>>
>>>
>>> So it reads:
>>>
>>> ^self select: [:each| seen addedNew: each]
>>>
>>> I like that how that reads.
>>>
>>> #added: wasn't bad - but I could see myself thinking it meant roughly 'was
>>> this value already in the collection'?  Which misses the add part and is
>>> backwards on the result (true if present, false if added).
>>>
>>> -cbc
>>>>
>>>>
>>>>>
>>>>> withoutDuplicates
>>>>>      "Answer a copy of the receiver that preserves order but eliminates
>>>>> any duplicates."
>>>>>      | seen |
>>>>>      seen := Set new: self size.
>>>>>      ^self select: [:each| seen added: each]
>>>>>
>>>>> messagesDo: aBlock
>>>>>      "Evaluate aBlock exactly once with all the message selectors sent
>>>>> by me."
>>>>>
>>>>>      | scanner aSet |
>>>>>      self isQuick ifTrue: [ ^self ].
>>>>>      scanner := InstructionStream on: self.
>>>>>      scanner scanFor: [ :x |
>>>>>              | selector |
>>>>>              (selector := scanner selectorToSendOrSelf) == scanner
>>>>> ifFalse: [
>>>>>                      ((aSet ifNil: [ aSet := IdentitySet new ]) added:
>>>>> selector) ifTrue: [
>>>>>                              aBlock value: selector ] ].
>>>>>              false "keep scanning" ]
>>>>>
>>>>>
>>>>>
>>>>>> On Jun 15, 2017 12:19 PM, "Eliot Miranda" <[hidden email]>
>>>>>> wrote:
>>>>>> Hi Tobias,
>>>>>>
>>>>>> On Thu, Jun 15, 2017 at 12:06 PM, Tobias Pape <[hidden email]>
>>>>>> wrote:
>>>>>>
>>>>>>> On 15.06.2017, at 20:30, Eliot Miranda <[hidden email]>
>>>>>>> wrote:
>>>>>>>
>>>>>>> Hi Levente, Hi Chris,
>>>>>>>
>>>>>>> On Wed, Jun 14, 2017 at 1:10 PM, Levente Uzonyi
>>>>>>> <[hidden email]> wrote:
>>>>>>> On Wed, 14 Jun 2017, [hidden email] wrote:
>>>>>>>
>>>>>>> Eliot Miranda uploaded a new version of Collections to project The
>>>>>>> Trunk:
>>>>>>> http://source.squeak.org/trunk/Collections-eem.756.mcz
>>>>>>>
>>>>>>> ==================== Summary ====================
>>>>>>>
>>>>>>> Name: Collections-eem.756
>>>>>>> Author: eem
>>>>>>> Time: 14 June 2017, 11:03:24.917631 am
>>>>>>> UUID: 8d7c03bc-1cdb-44c7-9173-10d50c0dae29
>>>>>>> Ancestors: Collections-eem.755
>>>>>>>
>>>>>>> Add SequenceableCollection>>withoutDuplicates for a more elegant fix
>>>>>>> to MailMessage>>to:
>>>>>>>
>>>>>>> =============== Diff against Collections-eem.755 ===============
>>>>>>>
>>>>>>> Item was added:
>>>>>>> + ----- Method: SequenceableCollection>>withoutDuplicates (in
>>>>>>> category 'copying') -----
>>>>>>> + withoutDuplicates
>>>>>>> +       "Answer a copy of the receiver that preserves order but
>>>>>>> eliminates any duplicates."
>>>>>>> +       | seen |
>>>>>>> +       seen := Set new: self size.
>>>>>>> +       ^self select: [:each|
>>>>>>> +                                 (seen includes: each)
>>>>>>> +                                       ifTrue: [false]
>>>>>>> +                                       ifFalse: [seen add: each.
>>>>>>> true]]!
>>>>>>>
>>>>>>> What a great opportunity to use #addNewElement::
>>>>>>>
>>>>>>>        ^self select: [ :each | seen addNewElement: each ]
>>>>>>>
>>>>>>> I love the functionality but I don't like the selector. It seems to
>>>>>>> imply that one must only add a new element.  So why not call this something
>>>>>>> like addIfAbsent: ?
>>>>>>>
>>>>>>> Here are some suggestions.  Votes?
>>>>>>> - don't change it; stick with addNewElement:
>>>>>>> - addIfAbsent:
>>>>>>> - ifAbsentAdd:
>>>>>>> - ifMissingAdd:
>>>>>>>
>>>>>>> I think I prefer ifAbsentAdd: cuz addIfAbsent: looks too much like a
>>>>>>> potential spelling error, and conflicts with typical ifAbsent: arguments
>>>>>>> supplying exception blocks.  But I could go with ifMissingAdd: because it is
>>>>>>> more distinctive.
>>>>>>
>>>>>> Well, we do have
>>>>>>
>>>>>>        Collection>>addIfNotPresent:
>>>>>>
>>>>>> So why invent a new one?
>>>>>>
>>>>>> Because addIfNotPresent: answers its argument and we need one that
>>>>>> answers whether the element was absent.  So alas addIfNotPresent: is not a
>>>>>> suitable candidate.
>>>>>>
>>>>>>
>>>>>> I think the important thing with #addNewElement: is that it returns
>>>>>> *whether* it added a new element
>>>>>> but then again it breaks the tradition of #add* returning its
>>>>>> argument…
>>>>>>
>>>>>> a very clear and very strange one that would reveal that a boolean is
>>>>>> returned would be #isAbsentAndIfSoAdd: …
>>>>>>
>>>>>> wasAbsentAdding: or ifWasAbsentAdding: would be less cumbersome but I
>>>>>> like something snappier that people will remember.  ifMissingAdd: looks good
>>>>>> because it doesn't conflict with the add*: methods answering their argument,
>>>>>> and the ifMissing implies the answer is true if the element wasn't already
>>>>>> present.
>>>>>>
>>>>>> Stéphane, can you live with ifMissingAdd: ?  Chris?
>>>>>>
>>>>>> Best regards
>>>>>>        -Tobias
>>>>>>
>>>>>>> _,,,^..^,,,_
>>>>>>> best, Eliot
>>>>>>
>>>>>> _,,,^..^,,,_
>>>>>> best, Eliot
>>>>>
>>>>> _,,,^..^,,,_
>>>>> best, Eliot
>>>>>
>>>>
>>>>
>>>
>>>
>>>
>>>
>>
>
>

Reply | Threaded
Open this post in threaded view
|

Re: The Trunk: Collections-eem.756.mcz

Chris Muller-4
In reply to this post by Chris Muller-4
> And, I didn't go with #ifMissingAdd: because it confuses the return value with the linguistic semantics -- e.g., if it returns true, does that mean it was "absent" or that it was added?  forcing me to look at the method implementation again anyway.

I think I confused this with a different one.  ifMissingAdd:
actually seems fine in terms of the return value semantics..  "true"
would mean, "yes", it was missing and added.

From there, saying we want to stay with the "Absent" terminology in the
API seems reasonable.

+1 for ifAbsentAdd:.

12