On Tue, 3 Feb 2015, commits@source.squeak.org wrote:
Eliot Miranda uploaded a new version of Collections to project The Trunk: http://source.squeak.org/trunk/Collections-eem.603.mcz
==================== Summary ====================
Name: Collections-eem.603 Author: eem Time: 3 February 2015, 12:06:59.618 pm UUID: 6521c82f-d8de-4c07-a754-3bc3a8667746 Ancestors: Collections-mt.602
Three fewer uses of isKindOf:
=============== Diff against Collections-mt.602 ===============
Item was changed: ----- Method: Dictionary>>= (in category 'comparing') ----- = aDictionary "Two dictionaries are equal if (a) they are the same 'kind' of thing. (b) they have the same set of keys. (c) for each (common) key, they have the same value"
self == aDictionary ifTrue: [ ^ true ].
- aDictionary isDictionary ifFalse: [^false].
- (aDictionary isKindOf: Dictionary) ifFalse: [^false]. self size = aDictionary size ifFalse: [^false]. self associationsDo: [:assoc| (aDictionary at: assoc key ifAbsent: [^false]) = assoc value ifFalse: [^false]]. ^true
!
Item was changed: ----- Method: SequenceableCollection>>hasEqualElements: (in category 'comparing') ----- hasEqualElements: otherCollection "Answer whether the receiver's size is the same as otherCollection's size, and each of the receiver's elements equal the corresponding element of otherCollection. This should probably replace the current definition of #= ."
| size |
- otherCollection isSequenceable ifFalse: [^ false].
- (otherCollection isKindOf: SequenceableCollection) ifFalse: [^ false].
This should either include an #isCollection check, or #isSequenceable should be moved up to Object.
Levente
(size := self size) = otherCollection size ifFalse: [^ false]. 1 to: size do: [:index | (self at: index) = (otherCollection at: index) ifFalse: [^ false]]. ^ true!
Item was changed: ----- Method: String>>subStrings: (in category 'converting') ----- subStrings: separators "Answer an array containing the substrings in the receiver separated by the elements of separators." | char result sourceStream subString | #Collectn. "Changed 2000/04/08 For ANSI <readableString> protocol."
- (separators isString or:[separators allSatisfy: [:element | element isCharacter]]) ifFalse:
[^ self error: 'separators must be Characters.'].
- (separators isString or:[separators allSatisfy: [:element | element isKindOf: Character]])
ifFalse: [^ self error: 'separators must be Characters.'].
On Tue, Feb 3, 2015 at 12:22 PM, Levente Uzonyi leves@elte.hu wrote:
On Tue, 3 Feb 2015, commits@source.squeak.org wrote:
Eliot Miranda uploaded a new version of Collections to project The Trunk:
http://source.squeak.org/trunk/Collections-eem.603.mcz
==================== Summary ====================
Name: Collections-eem.603 Author: eem Time: 3 February 2015, 12:06:59.618 pm UUID: 6521c82f-d8de-4c07-a754-3bc3a8667746 Ancestors: Collections-mt.602
Three fewer uses of isKindOf:
=============== Diff against Collections-mt.602 ===============
Item was changed: ----- Method: Dictionary>>= (in category 'comparing') ----- = aDictionary "Two dictionaries are equal if (a) they are the same 'kind' of thing. (b) they have the same set of keys. (c) for each (common) key, they have the same value"
self == aDictionary ifTrue: [ ^ true ].
aDictionary isDictionary ifFalse: [^false].
(aDictionary isKindOf: Dictionary) ifFalse: [^false]. self size = aDictionary size ifFalse: [^false]. self associationsDo: [:assoc| (aDictionary at: assoc key ifAbsent: [^false]) = assocvalue ifFalse: [^false]]. ^true
!
Item was changed: ----- Method: SequenceableCollection>>hasEqualElements: (in category 'comparing') ----- hasEqualElements: otherCollection "Answer whether the receiver's size is the same as otherCollection's size, and each of the receiver's elements equal the corresponding element of otherCollection. This should probably replace the current definition of #= ."
| size |
otherCollection isSequenceable ifFalse: [^ false].
(otherCollection isKindOf: SequenceableCollection) ifFalse: [^false].
This should either include an #isCollection check, or #isSequenceable should be moved up to Object.
Oops. Good catch. Which would you do? I favour the latter.
Levente
(size := self size) = otherCollection size ifFalse: [^ false].1 to: size do: [:index | (self at: index) = (otherCollection at: index) ifFalse:[^ false]]. ^ true!
Item was changed: ----- Method: String>>subStrings: (in category 'converting') ----- subStrings: separators "Answer an array containing the substrings in the receiver separated by the elements of separators." | char result sourceStream subString | #Collectn. "Changed 2000/04/08 For ANSI <readableString> protocol."
(separators isString or:[separators allSatisfy: [:element |element isCharacter]]) ifFalse:
[^ self error: 'separators must be Characters.'].
(separators isString or:[separators allSatisfy: [:element |element isKindOf: Character]])
ifFalse: [^ self error: 'separators must be Characters.']. sourceStream := ReadStream on: self. result := OrderedCollection new. subString := String new. [sourceStream atEnd] whileFalse: [char := sourceStream next. (separators includes: char) ifTrue: [subString notEmpty ifTrue: [result add:subString copy. subString := String new]] ifFalse: [subString := subString , (String with: char)]]. subString notEmpty ifTrue: [result add: subString copy]. ^ result asArray!
isSequenceable is a term that refers to a particular *kind* of Collection, a sequenceable one.
Therefore, IMO, I am unable to think of any more clear and explicit way of expressing that than "isKindOf: SequenceableCollection"...
On Tue, Feb 3, 2015 at 8:59 PM, Eliot Miranda eliot.miranda@gmail.com wrote:
On Tue, Feb 3, 2015 at 12:22 PM, Levente Uzonyi leves@elte.hu wrote:
On Tue, 3 Feb 2015, commits@source.squeak.org wrote:
Eliot Miranda uploaded a new version of Collections to project The Trunk: http://source.squeak.org/trunk/Collections-eem.603.mcz
==================== Summary ====================
Name: Collections-eem.603 Author: eem Time: 3 February 2015, 12:06:59.618 pm UUID: 6521c82f-d8de-4c07-a754-3bc3a8667746 Ancestors: Collections-mt.602
Three fewer uses of isKindOf:
=============== Diff against Collections-mt.602 ===============
Item was changed: ----- Method: Dictionary>>= (in category 'comparing') ----- = aDictionary "Two dictionaries are equal if (a) they are the same 'kind' of thing. (b) they have the same set of keys. (c) for each (common) key, they have the same value"
self == aDictionary ifTrue: [ ^ true ].
aDictionary isDictionary ifFalse: [^false].
(aDictionary isKindOf: Dictionary) ifFalse: [^false]. self size = aDictionary size ifFalse: [^false]. self associationsDo: [:assoc| (aDictionary at: assoc key ifAbsent: [^false]) = assocvalue ifFalse: [^false]]. ^true
!
Item was changed: ----- Method: SequenceableCollection>>hasEqualElements: (in category 'comparing') ----- hasEqualElements: otherCollection "Answer whether the receiver's size is the same as otherCollection's size, and each of the receiver's elements equal the corresponding element of otherCollection. This should probably replace the current definition of #= ."
| size |
otherCollection isSequenceable ifFalse: [^ false].
(otherCollection isKindOf: SequenceableCollection) ifFalse: [^false].
This should either include an #isCollection check, or #isSequenceable should be moved up to Object.
Oops. Good catch. Which would you do? I favour the latter.
Levente
(size := self size) = otherCollection size ifFalse: [^ false]. 1 to: size do: [:index | (self at: index) = (otherCollection at: index) ifFalse:[^ false]]. ^ true!
Item was changed: ----- Method: String>>subStrings: (in category 'converting') ----- subStrings: separators "Answer an array containing the substrings in the receiver separated by the elements of separators." | char result sourceStream subString | #Collectn. "Changed 2000/04/08 For ANSI <readableString> protocol."
(separators isString or:[separators allSatisfy: [:element |element isCharacter]]) ifFalse:
[^ self error: 'separators must be Characters.'].
(separators isString or:[separators allSatisfy: [:element |element isKindOf: Character]])
ifFalse: [^ self error: 'separators must beCharacters.']. sourceStream := ReadStream on: self. result := OrderedCollection new. subString := String new. [sourceStream atEnd] whileFalse: [char := sourceStream next. (separators includes: char) ifTrue: [subString notEmpty ifTrue: [result add: subString copy. subString := String new]] ifFalse: [subString := subString , (String with: char)]]. subString notEmpty ifTrue: [result add: subString copy]. ^ result asArray!
-- best, Eliot
On Wed, Feb 4, 2015 at 9:07 AM, Chris Muller asqueaker@gmail.com wrote:
isSequenceable is a term that refers to a particular *kind* of Collection, a sequenceable one.
Therefore, IMO, I am unable to think of any more clear and explicit way of expressing that than "isKindOf: SequenceableCollection"...
self isCollection and: [self isSequenceable] is better. isKinfOf: is a) not object-oriented as it forces an argument to be in a particular hierarchy rather than having a particular interface, and b) is horribly inefficient, causing a potentially long search of an object's class hierarchy. isKindOf: doesn't just smell, it stinks.
So what do you prefer Chris, making isSequenceable an Object method too, or using self isCollection and: [self isSequenceable]? I like the former because its simple, but you might have valid objections to extending Object. That's why I'm canvassing opinions. I wont stop nuking isKindOf:'s as I see them though ;-)
On Tue, Feb 3, 2015 at 8:59 PM, Eliot Miranda eliot.miranda@gmail.com wrote:
On Tue, Feb 3, 2015 at 12:22 PM, Levente Uzonyi leves@elte.hu wrote:
On Tue, 3 Feb 2015, commits@source.squeak.org wrote:
Eliot Miranda uploaded a new version of Collections to project The
Trunk:
http://source.squeak.org/trunk/Collections-eem.603.mcz
==================== Summary ====================
Name: Collections-eem.603 Author: eem Time: 3 February 2015, 12:06:59.618 pm UUID: 6521c82f-d8de-4c07-a754-3bc3a8667746 Ancestors: Collections-mt.602
Three fewer uses of isKindOf:
=============== Diff against Collections-mt.602 ===============
Item was changed: ----- Method: Dictionary>>= (in category 'comparing') ----- = aDictionary "Two dictionaries are equal if (a) they are the same 'kind' of thing. (b) they have the same set of keys. (c) for each (common) key, they have the same value"
self == aDictionary ifTrue: [ ^ true ].
aDictionary isDictionary ifFalse: [^false].
(aDictionary isKindOf: Dictionary) ifFalse: [^false]. self size = aDictionary size ifFalse: [^false]. self associationsDo: [:assoc| (aDictionary at: assoc key ifAbsent: [^false]) = assocvalue ifFalse: [^false]]. ^true
!
Item was changed: ----- Method: SequenceableCollection>>hasEqualElements: (in category 'comparing') ----- hasEqualElements: otherCollection "Answer whether the receiver's size is the same as otherCollection's size, and each of the receiver's elements equal the
corresponding
element of otherCollection. This should probably replace the current definition of #= ." | size |
otherCollection isSequenceable ifFalse: [^ false].
(otherCollection isKindOf: SequenceableCollection) ifFalse: [^false].
This should either include an #isCollection check, or #isSequenceable should be moved up to Object.
Oops. Good catch. Which would you do? I favour the latter.
Levente
(size := self size) = otherCollection size ifFalse: [^ false]. 1 to: size do: [:index | (self at: index) = (otherCollection at: index) ifFalse:[^ false]]. ^ true!
Item was changed: ----- Method: String>>subStrings: (in category 'converting') ----- subStrings: separators "Answer an array containing the substrings in the receiver separated by the elements of separators." | char result sourceStream subString | #Collectn. "Changed 2000/04/08 For ANSI <readableString> protocol."
(separators isString or:[separators allSatisfy: [:element |element isCharacter]]) ifFalse:
[^ self error: 'separators must be Characters.'].
(separators isString or:[separators allSatisfy: [:element |element isKindOf: Character]])
ifFalse: [^ self error: 'separators must beCharacters.']. sourceStream := ReadStream on: self. result := OrderedCollection new. subString := String new. [sourceStream atEnd] whileFalse: [char := sourceStream next. (separators includes: char) ifTrue: [subString notEmpty ifTrue: [result add: subString copy. subString := String new]] ifFalse: [subString := subString , (String with: char)]]. subString notEmpty ifTrue: [result add: subString copy]. ^ result asArray!
-- best, Eliot
On 04.02.2015, at 18:17, Eliot Miranda eliot.miranda@gmail.com wrote:
isKindOf: doesn't just smell, it stinks.
ACK.
On Wed, Feb 4, 2015 at 9:07 AM, Chris Muller asqueaker@gmail.com wrote:
isSequenceable is a term that refers to a particular *kind* of Collection, a sequenceable one.
Therefore, IMO, I am unable to think of any more clear and explicit way of expressing that than "isKindOf: SequenceableCollection"...
self isCollection and: [self isSequenceable] is better. isKinfOf: is a) not object-oriented as it forces an argument to be in a particular hierarchy rather than having a particular interface, and b) is horribly inefficient, causing a potentially long search of an object's class hierarchy. isKindOf: doesn't just smell, it stinks.
Yes, and class-testing via #class as well, for the same reasons. But sometimes we really do want to know whether we have a _particular implementation_ of a Dictionary, not just dictionary behaviors and API's.
I'm not saying I'm against your change, I'm just saying that I feel this kind of "cleaning" should be done with deliberation and careful consideration, not whimsically. Today all of the #= implementations in the system are based on type coherence, except for this one which was just changed. It's easier than it seems for semantics to be subtly changed.
For example, I have several of my own dictionary implementations which do not inherit from Dictionary; but if they implement #isDictionary for a different reason, I will now get a different answer from #= depending on which I ask it; (aDictionary = theirSpecialDictionary) vs. (theirSpecialDictionary = aDictionary) might produce different results if SpecialDictionary is doing the conventional "type" check.
So what do you prefer Chris, making isSequenceable an Object method too, or using self isCollection and: [self isSequenceable]? I like the former because its simple, but you might have valid objections to extending Object. That's why I'm canvassing opinions. I wont stop nuking isKindOf:'s as I see them though ;-)
I prefer the latter because its asking if its a particular sub-types require a super-type check first. I think if you want simplicity you should implement #isSequenceableCollection on Object to return the "self isCollection and: [self isSequenceable]".
Because another consideration is consumption of the method namespace on Object with simple terms that then become unavailable to applications. The most egregious example I can think of is Object>>#name. It means an app implement #name without overriding the one in Object. But every app has things that have #names. There is a remote possibility that an app might want to implement the #isSequenceable for a application-specific purpose..
I vote for:
"... xx isCollection and: [xx isSequenceable] ..."
Best, Marcel
-- View this message in context: http://forum.world.st/The-Trunk-Collections-eem-603-mcz-tp4803479p4803678.ht... Sent from the Squeak - Dev mailing list archive at Nabble.com.
On 04-02-2015, at 9:20 AM, Marcel Taeumel marcel.taeumel@student.hpi.uni-potsdam.de wrote:
I vote for:
"... xx isCollection and: [xx isSequenceable] …"
And I’d prefer a simple isSequenceableCollection over that. I don’t actually like any of them because there is almost always A Better Way(™) hidden in there somewhere, but at least a very specific method is easy to track down and replace when said better way is discovered. They’re all over the place in nuScratch but the performance improvements simply from removing multiple isKindOf: sends within large loops are well worth the slight stinkiness.
tim -- tim Rowledge; tim@rowledge.org; http://www.rowledge.org/tim "How many Slavers does it take to change a lightbulb?” "Dunno. How susceptible are lightbulbs to telepathy?"
On Tue, 3 Feb 2015, Eliot Miranda wrote:
Oops. Good catch. Which would you do? I favour the latter.
I would not do the latter, because #isSequenceable is too general for Object. I would either add a new method - #isSequenceableCollection - or use the combination of #isCollection and #isSequenceable.
Levente
squeak-dev@lists.squeakfoundation.org
-
Chris Muller -
Chris Muller -
Eliot Miranda -
Levente Uzonyi -
Marcel Taeumel -
tim Rowledge -
Tobias Pape