OrderedCollection as an instance variable
I'm implementing an instance variable as an OrderedCollection, and am doing something idiotic. I have an Artist class. An Artist can have many tracks: tracks ^ tracks ifNil: [ self tracks: OrderedCollection new ] tracks: anObject tracks := anObject Doing something like: a := Artist new. t := Track new. a tracks add: t Gives me the error: Artist(Object)>>doesNotUnderstand: #add: Once I get past that error, which will evaluate `a tracks`, I can run this. I get why it works the second time, but I don't get why it thinks the instance of Artist is the receiver of add: on the first run. Thanks! ---- peace, sergio photographer, journalist, visionary Public Key: http://bit.ly/29z9fG0 #BitMessage BM-NBaswViL21xqgg9STRJjaJaUoyiNe2dV http://www.codeandmusic.com http://www.twitter.com/sergio_101 http://www.facebook.com/sergio101
because your #tracks: returns self, not the collection value
On 24 Jul 2019, at 16:46, sergio ruiz <sergio.rrd@gmail.com> wrote:
I'm implementing an instance variable as an OrderedCollection, and am doing something idiotic.
I have an Artist class.
An Artist can have many tracks:
tracks ^ tracks ifNil: [ self tracks: OrderedCollection new ]
tracks: anObject tracks := anObject
Doing something like:
a := Artist new. t := Track new. a tracks add: t
Gives me the error:
Artist(Object)>>doesNotUnderstand: #add:
Once I get past that error, which will evaluate `a tracks`, I can run this.
I get why it works the second time, but I don't get why it thinks the instance of Artist is the receiver of add: on the first run.
Thanks! ---- peace, sergio photographer, journalist, visionary
Public Key: http://bit.ly/29z9fG0 #BitMessage BM-NBaswViL21xqgg9STRJjaJaUoyiNe2dV http://www.codeandmusic.com http://www.twitter.com/sergio_101 http://www.facebook.com/sergio101
Oh! how about this: tracks: anObject tracks := anObject. ^ tracks ?
On Jul 24, 2019, at 10:48 AM, Sven Van Caekenberghe <sven@stfx.eu> wrote:
because your #tracks: returns self, not the collection value
---- peace, sergio photographer, journalist, visionary Public Key: http://bit.ly/29z9fG0 #BitMessage BM-NBaswViL21xqgg9STRJjaJaUoyiNe2dV http://www.codeandmusic.com http://www.twitter.com/sergio_101 http://www.facebook.com/sergio101
You should change your #tracks method to tracks ^tracks ifNil:[tracks:=OrderedCollection new] or tracks tracks ifNil:[self initializeTracks] ^tracks initializeTracks tracks:=OrderedCollection new. Then you don't have to worry about the implementation of #tracks: changing in the future if you want to do something else with/in it. -- Sent from: http://forum.world.st/Pharo-Smalltalk-Users-f1310670.html
hmm⦠maybe this is cleaner.. tracks tracks ifNil: [ self tracks: OrderedCollection new ]. ^ tracks
because your #tracks: returns self, not the collection value
---- peace, sergio photographer, journalist, visionary Public Key: http://bit.ly/29z9fG0 #BitMessage BM-NBaswViL21xqgg9STRJjaJaUoyiNe2dV http://www.codeandmusic.com http://www.twitter.com/sergio_101 http://www.facebook.com/sergio101
On 24 Jul 2019, at 17:30, sergio ruiz <sergio.rrd@gmail.com> wrote:
hmmâ¦
maybe this is cleaner..
tracks tracks ifNil: [ self tracks: OrderedCollection new ]. ^ tracks
I write these methods as tracks ^tracks ifNil: [ tracks := OrderedCollection new ]. - one line - does not mix using accusers and not using accessors. Marcus
tracks ^tracks ifNil: [ tracks := OrderedCollection new ].
- one line - does not mix using accusers and not using accessors.
Okay, I totally get the subtlety now. I donât even need an accessor. Thanks! ---- peace, sergio photographer, journalist, visionary Public Key: http://bit.ly/29z9fG0 #BitMessage BM-NBaswViL21xqgg9STRJjaJaUoyiNe2dV http://www.codeandmusic.com http://www.twitter.com/sergio_101 http://www.facebook.com/sergio101
I will add that I prefer the static model to tell the truth about its data types. So, I prefer having an #initialize method which ensures the new object is created in a self-consistent way and anyone looking at it in an inspector will see useful and correct information. Leaving instance variables uninitialized is a excellent way to generate Does Not Understand errors (perhaps not in the short-term, but definitely in the long-term). On Wed, Jul 24, 2019 at 8:39 AM sergio ruiz <sergio.rrd@gmail.com> wrote:
tracks ^tracks ifNil: [ tracks := OrderedCollection new ].
- one line - does not mix using accusers and not using accessors.
Okay, I totally get the subtlety now. I donât even need an accessor.
Thanks!
---- peace, sergio photographer, journalist, visionary
Public Key: http://bit.ly/29z9fG0 #BitMessage BM-NBaswViL21xqgg9STRJjaJaUoyiNe2dV http://www.codeandmusic.com http://www.twitter.com/sergio_101 http://www.facebook.com/sergio101
I agree with that, but in some cases, like reading objects from an ORM, you would be creating lots of instances of objects that are going to be discarded right away. This would put no stress on GC since it would happen in the "new" space, but would consume more memory (although I never measured how much more). Esteban A. Maringolo On Wed, Jul 24, 2019 at 1:14 PM Richard Sargent <richard.sargent@gemtalksystems.com> wrote:
I will add that I prefer the static model to tell the truth about its data types. So, I prefer having an #initialize method which ensures the new object is created in a self-consistent way and anyone looking at it in an inspector will see useful and correct information. Leaving instance variables uninitialized is a excellent way to generate Does Not Understand errors (perhaps not in the short-term, but definitely in the long-term).
On Wed, Jul 24, 2019 at 8:39 AM sergio ruiz <sergio.rrd@gmail.com> wrote:
tracks ^tracks ifNil: [ tracks := OrderedCollection new ].
- one line - does not mix using accusers and not using accessors.
Okay, I totally get the subtlety now. I donât even need an accessor.
Thanks!
---- peace, sergio photographer, journalist, visionary
Public Key: http://bit.ly/29z9fG0 #BitMessage BM-NBaswViL21xqgg9STRJjaJaUoyiNe2dV http://www.codeandmusic.com http://www.twitter.com/sergio_101 http://www.facebook.com/sergio101
A book that looks explores implementations like that and that always had a place on my desk is this https://www.amazon.ca/Smalltalk-Style-Suzanne-Skublics/dp/0131655493 I can highly recommend it. Sebastian On 2019-07-24 8:32 a.m., Marcus Denker wrote:
On 24 Jul 2019, at 17:30, sergio ruiz <sergio.rrd@gmail.com> wrote:
hmm???
maybe this is cleaner..
tracks tracks ifNil: [ self tracks: OrderedCollection new ]. ^ tracks
I write these methods as
tracks ^tracks ifNil: [ tracks := OrderedCollection new ].
- one line - does not mix using accusers and not using accessors.
Marcus
On 24. 7. 2019 17:30, sergio ruiz wrote:
hmm???
maybe this is cleaner..
tracks tracks ifNil: [ self tracks: OrderedCollection new ]. ^ tracks
IMO, return ifNil: value is an understood and used idiom, so I'd say ^ tracks ifNil: [ tracks := ... ] is the most clean way. Maybe look at sender of ifNil: and see for yourself which is the most idiomatic way to do lazy getter in the current image (there will be lots of these usages there). Herby
because your #tracks: returns self, not the collection value
---- peace, sergio photographer, journalist,??visionary
Public Key: http://bit.ly/29z9fG0 #BitMessage BM-NBaswViL21xqgg9STRJjaJaUoyiNe2dV http://www.codeandmusic.com http://www.twitter.com/sergio_101 http://www.facebook.com/sergio101
Comment 1. You probably meant to write tracks: aCollection ^tracks := aCollection Comment 2. This is a poor design. As it is, any object can replace the tracks of an artist with *anything*. And even without doing that, any object can add and remove items to an artist's tracks, even if the added items are not Tracks. There are a number of OO design principles, notably the Law of Demeter and of course the GRASP patterns, which basically say "never mutate another object's parts, ask IT to do the mutation." I have come to regard the self-encapsulation "pattern" as an anti-pattern. This is Smalltalk heresy, admittedly, but it doesn't so much break (real) encapsulation as set it on fire, defenestrate it, and urinate on the smouldering remains. So this is the way I'd do it. Object subclass: #Artist instanceVariableNames: 'tracks' methods for: 'initialising' initialize tracks := OrderedCollection new. ^self methods for: 'checking' invariant ^(tracks isMemberOf: OrderedCollection) and: [ tracks allSatisfy: [:each | each isKindOf: Track]] methods for: 'accessing tracks' addTrack: aTrack. tracks add: aTrack. ^track tracksDo: doBlock tracks do: doBlock with no way for outside code to accesss 'tracks' directly. On Thu, 25 Jul 2019 at 02:47, sergio ruiz <sergio.rrd@gmail.com> wrote:
I'm implementing an instance variable as an OrderedCollection, and am doing something idiotic.
I have an Artist class.
An Artist can have many tracks:
tracks ^ tracks ifNil: [ self tracks: OrderedCollection new ]
tracks: anObject tracks := anObject
Doing something like:
a := Artist new. t := Track new. a tracks add: t
Gives me the error:
Artist(Object)>>doesNotUnderstand: #add:
Once I get past that error, which will evaluate `a tracks`, I can run this.
I get why it works the second time, but I don't get why it thinks the instance of Artist is the receiver of add: on the first run.
Thanks! ---- peace, sergio photographer, journalist, visionary
Public Key: http://bit.ly/29z9fG0 #BitMessage BM-NBaswViL21xqgg9STRJjaJaUoyiNe2dV http://www.codeandmusic.com http://www.twitter.com/sergio_101 http://www.facebook.com/sergio101
On 25. 7. 2019 4:27, Richard O'Keefe wrote:
Comment 2. ?? This is a poor design.?? As it is, any object can replace the tracks of an artist ?? with *anything*.?? And even without doing that, any object can add and remove ?? items to an artist's tracks, even if the added items are not Tracks. ?? There are a number of OO design principles, notably the Law of Demeter and ?? of course the GRASP patterns, which basically say "never mutate another ?? object's parts, ask IT to do the mutation."
As outlined nicely in this classic: https://alanknightsblog.blogspot.com/2011/10/principles-of-oo-design-or-ever....
participants (9)
-
Esteban Maringolo -
Herby VojÄÃk -
Marcus Denker -
Paul DeBruicker -
Richard O'Keefe -
Richard Sargent -
Sebastian Heidbrink -
sergio ruiz -
Sven Van Caekenberghe