The Trunk: Traits-nice.239.mcz

Nicolas Cellier uploaded a new version of Traits to project The Trunk:
http://source.squeak.org/trunk/Traits-nice.239.mcz

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

Name: Traits-nice.239
Author: nice
Time: 21 October 2009, 2:55 am
UUID: da00b4a5-33ee-4d7e-bdf5-41e2d7ff90ae
Ancestors: Traits-nice.238

correct a missing #asSet after #keys refactoring

=============== Diff against Traits-nice.238 ===============

Item was changed:
  ----- Method: TraitBehavior>>allSelectors (in category 'accessing method dictionary') -----
  allSelectors
+ ^ self selectors asSet!
- ^ self selectors!

Again I think you should push the asSet into selectors.

Hi Eliot,
This is experimental, not guaranteed bugfree.
Indeed, you cannot #add: to nor #remove: from an Array... and elements
won't be unique.
My trunk image did not freeze because I analyzed senders of #selectors in it.
But sure, this might break foreign code...
So yes, it deserves discussion.

What do others think ?

Fortunately, it is very easy to retract this change...
...just have to check if aClass selectors sort is not sent directly.
I don't think it is essential and I won't fight to defend it.


Note that I also changed classVarNames as a sorted Array.
I think this is a more interesting feature, because mostly used this way.

Do others agree on this one ?

Nicolas


2009/10/21 Eliot Miranda <[hidden email]>:

On 21.10.2009, at 20:11, Nicolas Cellier wrote:

I agree with Eliot. Keep the changes minimal - so just replace "keys"  
with "keys asSet" where necessary.

- Bert -

On 21-Oct-09, at 11:24 AM, Bert Freudenberg wrote:

Me too. #selectors should return a Set.

Colin

2009/10/21 Colin Putney <[hidden email]>:
Though I found no such usage in MC1.0 :)
But I didn't inquire MC2.0, DeltaStreams, Etoys nor any other
reflexion crunching packages...

BTW, it would be great to have universal code analysis tools spreading
over a public code database, not just my image. It dream of such
database served through web pages...

Nicolas

2009/10/21 Nicolas Cellier <[hidden email]>:
Last thing, before retracting this change, I give hereafter a little
rationale of my choice:

There were 92 senders of selectors in trunk, I classified into three categories

1) Some senders performing add: or remove:
If I recall, only 3 of them were I placed some #asSet protection.

2) Some senders expecting unique keys :
but hey, selectors have unique elements because methodDictionary keys
are unique...
Only those aggregating with other selectors care, but most of them
explicitely addAll: to an empty IdentitySet...

3) Every others did use an Array compatible message like do: includes:
select: collect: asSortedCollection etc...


Then I analyzed the cost of selectors:

1) Array will perform better than Set for all these selectors, but includes:.
2) Most direct senders of selector includes: SHOULD better be written
includesSelector:
Rationale: a single message send that will test includesKey: rather
than two creating unecessarily an intermediate Set
3) 95% of senders would now uselessly perform an additional  #asSet operation

So my analyze was that I would personnaly buy this little optimization
with the drawback of having to rewrite 5% of senders at most.
But of course, I don't offer to rewritte your code, so I understand
your opinion may differ :)
And of course I agree that optimized code that breaks has no value.
That's just a matter of tradeoffs.

Nicolas