Class Encapsulation Errors in Python 2.3.3

[Tim Henderson]

Hi i have some errors in my code that i can't find the reason for here
is the error

i have 3 classes:
-Song
-Album
-Artist

Song has 3 pieces of data with getter and setter meathods:
-Name | i.e. song name
-Album
-Artist

Album has 3 pieces of data:
-Songs | a list of Song() instances
-Name
-Artist

Artist has 4 pieces of Data:
-Name
-Songs | a list of Song() instances
-Albums |a list of Album() instances
-unknownAlbum | the album a song is put into if it doesn't have an
album


now the problem arises when i add songs into multiple albums like
this:
Code:
------------------------------------------------
s = Song('song 1')
s2 = Song('song 2')

a = Album('hey')
a.addSong(s)
ab = Album('yeayea')
print a
print ab
ab.addSong(s2)
print
print a
print ab
------------------------------------------------
Output:
************************************************
hey song 1 hey |
yeayea song 1 hey |

hey song 1 hey | song 2 yeayea |
yeayea song 1 hey | song 2 yeayea |
************************************************

the "hey" album has "song 1" as it is suppose to however so does
"yeayea" even though when "song 1" was put in the album "hey" 'yeayea'
didn't even exist yet.

Why is this happening. I checked the actually memory refrence for each
instance and they are different, these supposidly isolated different
intances of clases are linked for some inexplicable reason.

Does any one know why this is happening and how i can fix it?

cheers
Tim Henderson

 

[Jeff Shannon]

Hi i have some errors in my code that i can't find the reason for here
is the error
[...]
the "hey" album has "song 1" as it is suppose to however so does
"yeayea" even though when "song 1" was put in the album "hey" 'yeayea'
didn't even exist yet.

I can think of two likely reasons offhand.

The first is that you're storing the songlist as a class attribute,
rather than as an instance attribute. The second is that you're using a
mutable default value in a method call. (I'm guessing that it's
probably this second reason.)

Do you have something like this?

class Album:
def __init__(self, songs=[]):
self.songs = songs
# ...

The problem here is that default values in functions/methods are
evaluated at *compile* time. That means that this creates a single
empty list, which is used as the default value for *every* Album
instance. When you mutate that list, then every instance with a
reference to that list shows the changes.

The other possibility (class attribute rather than instance attribute)
would happen if you have something like this:

class Album:
songs = []
def __init__(self, ...):
# ...

In this case, again, you have a single empty list that will end up being
shared by all Album instances.

What you probably want to do in this case is this:

class Album:
def __init__(self, songs=None)
if songs is None:
songs = []
self.songs = songs
# ...

This will allow you to have an optional songs parameter in the
initializer, but still ensure that every instance gets an independent
empty list if nothing's passed in.

Jeff Shannon
Technician/Programmer
Credit International

 

[Shalabh Chaturvedi]

Tim Henderson wrote:

now the problem arises when i add songs into multiple albums like
this:
Code:
------------------------------------------------
s = Song('song 1')
s2 = Song('song 2')

a = Album('hey')
a.addSong(s)
ab = Album('yeayea')
print a
print ab
ab.addSong(s2)
print
print a
print ab
------------------------------------------------
Output:
************************************************
hey song 1 hey |
yeayea song 1 hey |

hey song 1 hey | song 2 yeayea |
yeayea song 1 hey | song 2 yeayea |
************************************************

<snip>

Defnition of addSong() would help in determining the real cause. Taking
a guess here but it looks like the 'list' you store the songs in is
shared across all albums. Typically this happens if you define the list
in the class itself making it a class attribute - and not in __init__().
Please show us the code of Album to help you further.

Shalabh

 

[Dan Perl]

Please post the implementation of the Album class. The problem is coming
probably from the way the Songs list is defined or initialized. That is,
the definition or the initialization probably cause the list object that is
assigned to Songs to be shared by all the Album instances. But I (and
likely no one else) can say exactly what the problem is without seing the
code.

Dan

 

[Duncan Booth]

The problem here is that default values in functions/methods are
evaluated at *compile* time.

Actually, no. The default values in functions/methods are evaluated when
the def statement is *executed* not when anything is compiled. If you
execute the def multiple times the default values are recalculated each
time.

 

[Carlos Ribeiro]

Actually, no. The default values in functions/methods are evaluated when
the def statement is *executed* not when anything is compiled. If you
execute the def multiple times the default values are recalculated each
time.

Be careful default values are calculated when the def statement is
executed.... which is not the same as to say that they are calculated
every time the function is called.

To explain it better. In a conventional Python program, the def
statement is executed only once, when a module is loaded, for example.
In this case, default values are stored as attributes of the function
object. In this case, the value of the default never changes.

*If* for some reason your code runs over the def statement multiple
times, *then* a new function object will be created with new default
values. For example, this can happen if you force-reload a module, or
def the function inside a for loop (weird but it works):

for i in (1,2,3):
def print_i(x=i):
print x,i
3 0

--
Carlos Ribeiro
Consultoria em Projetos
blog: http://rascunhosrotos.blogspot.com
blog: http://pythonnotes.blogspot.com
mail: (e-mail address removed)
mail: (e-mail address removed)

 

[Duncan Booth]

wrote: > Jeff Shannon wrote: > > > The problem here is that default values
in functions/methods are > > evaluated at *compile* time. > > Actually,
no. The default values in functions/methods are evaluated when > the def
statement is *executed* not when anything is compiled. If you > execute
the def multiple times the default values are recalculated each > time.

Be careful default values are calculated when the def statement is
executed.... which is not the same as to say that they are calculated
every time the function is called.

<rest of explanation snipped>

I actually thought I had stated the case fairly clearly, but I
suppose understanding what you think you have said isn't the same as
communicating it to the audience.

 

[Tim Henderson]

Here is a partial implementation, i don't have the code with me but
this is how the songs list is made

code:
-------------------------------------------
class Album:

name = ''
songs = []
artist = ''

def __init__(self, name, artist):

self.name = name
self.artist = artist
-------------------------------------------

after reading the code i came up with a possible solution:

possible code:
-------------------------------------------
class Album:

name = ''
songs = False
artist = ''

def __init__(self, name, artist):

self.name = name
self.artist = artist
if songs == False:
songs = []
------------------------------------------


i will test this when i get home, and update the thread with results

cheers
Tim Henderson

 

[Jeff Shannon]

Jeff Shannon wrote:




Actually, no. The default values in functions/methods are evaluated when
the def statement is *executed* not when anything is compiled. If you
execute the def multiple times the default values are recalculated each
time.

Hm, true, I misspoke. Perhaps it's clearest to say that the default
values are evaluated when the function/method object is *created*, not
when it is called.

Functions are created when the def statement is executed; this is
usually one of the first things that happens when a simple module/script
is imported/run, but it can happen at other times as well. Referring to
the creation of function objects as compilation, as I did, is
technically inaccurate (compilation being the conversion of text source
code to bytecode, which is later executed). I do know the difference,
but tend to conflate them out of laziness in cases where the distinction
has little practical difference. (In this case, practical differences
are usually only significant when function/class definitions are dynamic
and/or some sort of deep magic is being employed, neither of which are
typical for me to need...) But I should indeed be more careful about
how I state that, because while *I* know what I meant, it doesn't follow
that someone else reading this and looking for guidance won't get bitten
by my laziness.

Jeff Shannon
Technician/Programmer
Credit International

 

[Jeff Shannon]

Here is a partial implementation, i don't have the code with me but
this is how the songs list is made

code:
-------------------------------------------
class Album:

name = ''
songs = []
artist = ''

def __init__(self, name, artist):

self.name = name
self.artist = artist

Yes, this gives you class-level attributes, shared by all instances of
Album. In the case of name and artist, you're then creating
instance-level attributes ('self.name = name') which shadow the
class-level attribute. But you never bind 'songs' on the instance, so
you're just modifying the class-level (shared) songs list.

after reading the code i came up with a possible solution:

possible code:
-------------------------------------------
class Album:

name = ''
songs = False
artist = ''

def __init__(self, name, artist):

self.name = name
self.artist = artist
if songs == False:
songs = []

That'll almost work -- within __init__(), you need to refer to songs as
self.songs (both times). But why bother with creating the class-level
attributes? If you just set them in __init__(), all will be fine.

class Album:

def __init__(self, name, artist):

self.name = name
self.artist = artist
self.songs = []

will have the same net effect as your code, for the vast majority of
common purposes. (There are corner cases where having class-level
defaults is helpful, but I sincerely doubt that this is true for your
program.)

Jeff Shannon
Technician/Programmer
Credit International

 

[Dan Perl]

I fully agree with Jeff and I will make a couple of other small points.

I saw you meant to use 'False' as a default value for the class-level
attribute 'songs'. 'None' is the normal value for initializations like
that. It's a small point but it kind of stuck in my mind as being unusual.

If you still choose to use class-level attributes and to initialize them
with default values, you have to keep in mind that you will encounter this
kind of problem with all the initializations that are using complex types.
That means lists like in the case of 'songs', but also strings like in the
case of 'name' and 'artist'. Jeff pointed out in an earlier posting that
there are also other ways you could get into trouble like this and it should
be made clear that this is true for all the complex types, not only lists.

Dan

Here is a partial implementation, i don't have the code with me but
this is how the songs list is made

code:
-------------------------------------------
class Album:
name = ''
songs = []
artist = ''

def __init__(self, name, artist):
self.name = name
self.artist = artist

Yes, this gives you class-level attributes, shared by all instances of
Album. In the case of name and artist, you're then creating
instance-level attributes ('self.name = name') which shadow the
class-level attribute. But you never bind 'songs' on the instance, so
you're just modifying the class-level (shared) songs list.

after reading the code i came up with a possible solution:

possible code:
-------------------------------------------
class Album:
name = ''
songs = False
artist = ''

def __init__(self, name, artist):
self.name = name
self.artist = artist
if songs == False:
songs = []

That'll almost work -- within __init__(), you need to refer to songs as
self.songs (both times). But why bother with creating the class-level
attributes? If you just set them in __init__(), all will be fine.

class Album:

def __init__(self, name, artist):

self.name = name
self.artist = artist
self.songs = []

will have the same net effect as your code, for the vast majority of
common purposes. (There are corner cases where having class-level
defaults is helpful, but I sincerely doubt that this is true for your
program.)

Jeff Shannon
Technician/Programmer
Credit International

 

[Dan Perl]

Errata: instead of "complex types" please read "mutable types".

I fully agree with Jeff and I will make a couple of other small points.

I saw you meant to use 'False' as a default value for the class-level
attribute 'songs'. 'None' is the normal value for initializations like
that. It's a small point but it kind of stuck in my mind as being
unusual.

If you still choose to use class-level attributes and to initialize them
with default values, you have to keep in mind that you will encounter this
kind of problem with all the initializations that are using complex types.
That means lists like in the case of 'songs', but also strings like in the
case of 'name' and 'artist'. Jeff pointed out in an earlier posting that
there are also other ways you could get into trouble like this and it
should be made clear that this is true for all the complex types, not only
lists.

Dan

Here is a partial implementation, i don't have the code with me but
this is how the songs list is made

code:
-------------------------------------------
class Album:
name = ''
songs = []
artist = ''

def __init__(self, name, artist):
self.name = name
self.artist = artist

Yes, this gives you class-level attributes, shared by all instances of
Album. In the case of name and artist, you're then creating
instance-level attributes ('self.name = name') which shadow the
class-level attribute. But you never bind 'songs' on the instance, so
you're just modifying the class-level (shared) songs list.

after reading the code i came up with a possible solution:

possible code:
-------------------------------------------
class Album:
name = ''
songs = False
artist = ''

def __init__(self, name, artist):
self.name = name
self.artist = artist
if songs == False:
songs = []

That'll almost work -- within __init__(), you need to refer to songs as
self.songs (both times). But why bother with creating the class-level
attributes? If you just set them in __init__(), all will be fine.

class Album:

def __init__(self, name, artist):

self.name = name
self.artist = artist
self.songs = []

will have the same net effect as your code, for the vast majority of
common purposes. (There are corner cases where having class-level
defaults is helpful, but I sincerely doubt that this is true for your
program.)

Jeff Shannon
Technician/Programmer
Credit International

 

[Terry Reedy]

Here is a partial implementation, i don't have the code with me but
this is how the songs list is made

code:
-------------------------------------------
class Album:

name = ''
songs = []
artist = ''

Delete all three of these lines. The name and artist lines have no effect
since they are over-ridden by the local variables of the same name in
__init__. The songs line makes songs an attribute of the Album class,
which you do not want, instead of each Album instance, which you do.

def __init__(self, name, artist):

If you want artist to be optional, augment to ..., artist = ''

self.name = name
self.artist = artist

add
self.songs = []
now songs is an attribute of the album.

possible revision [snip]
if songs == False:
songs = []

This assignment makes songs a local variable in the init function, which is
again what you don't want. In fact, the test will fail since local var
songs will not have a value before the assignment. See proper fix above.

Terry J. Reedy

 

[Dan Perl]

Better make another correction before someone else totally embarrasses me.
I was wrong, strings are not mutable.

That immediately made me wonder why strings are immutable. But it turns out
that question has already been beaten to death, not too long ago
(http://groups.google.com/groups?hl=...m=DbqdnXN8NqpMxrfcRVn-pw%40comcast.com&rnum=2)

Dan

Errata: instead of "complex types" please read "mutable types".

I fully agree with Jeff and I will make a couple of other small points.

I saw you meant to use 'False' as a default value for the class-level
attribute 'songs'. 'None' is the normal value for initializations like
that. It's a small point but it kind of stuck in my mind as being
unusual.

If you still choose to use class-level attributes and to initialize them
with default values, you have to keep in mind that you will encounter
this kind of problem with all the initializations that are using complex
types. That means lists like in the case of 'songs', but also strings
like in the case of 'name' and 'artist'. Jeff pointed out in an earlier
posting that there are also other ways you could get into trouble like
this and it should be made clear that this is true for all the complex
types, not only lists.

Dan

Tim Henderson wrote:

Here is a partial implementation, i don't have the code with me but
this is how the songs list is made

code:
-------------------------------------------
class Album:
name = ''
songs = []
artist = ''

def __init__(self, name, artist):
self.name = name
self.artist = artist
-------------------------------------------


Yes, this gives you class-level attributes, shared by all instances of
Album. In the case of name and artist, you're then creating
instance-level attributes ('self.name = name') which shadow the
class-level attribute. But you never bind 'songs' on the instance, so
you're just modifying the class-level (shared) songs list.

after reading the code i came up with a possible solution:

possible code:
-------------------------------------------
class Album:
name = ''
songs = False
artist = ''

def __init__(self, name, artist):
self.name = name
self.artist = artist
if songs == False:
songs = []
------------------------------------------


That'll almost work -- within __init__(), you need to refer to songs as
self.songs (both times). But why bother with creating the class-level
attributes? If you just set them in __init__(), all will be fine.

class Album:

def __init__(self, name, artist):

self.name = name
self.artist = artist
self.songs = []

will have the same net effect as your code, for the vast majority of
common purposes. (There are corner cases where having class-level
defaults is helpful, but I sincerely doubt that this is true for your
program.)

Jeff Shannon
Technician/Programmer
Credit International

 

[Tim Henderson]

I have solved the problem, it was indeed declaring the songs list in
the wrong place. I moved the declaration and the songs list no longer
acts like a static variable. I am going to have to read up on how the
interpreter evaluates variables declared there.


below is the new code, In this code the songs list no longer act like
the same variable.

code:
------------------------------------------------------------------
class Song:

name = ''
artist = ''
album = ''

def __init__(self, name, artist='', album=''):

self.name = name
self.artist = artist
self.album = album


def setName(self, name):
self.name = name

def getName(self):
return self.name

def setArtist(self, artist):
self.artist = artist

def getArtist(self):
return self.artist

def setAlbum(self, album):
self.album = album

def getAlbum(self):
return self.album

def __str__(self):
return self.name + ' ' + self.artist + ' ' + self.album + '
'


class Album:

songs = False
name = ''
artist = ''

def __init__(self, name, artist=''):
if not self.songs: self.songs = []
self.name = name
self.artist = artist

def setName(self, name):

self.name = name
for x in songs:
x.setAlbum(self.name)

def getName(self):
return self.name

def setArtist(self, artist):
self.artist = artist
for x in self.songs:
x.setArtist(self.artist)

def getArtist(self):
return self.artist

def addSong(self, song): #must be song class
#print 'NAME: ' + self.name
song.setAlbum(self.name)
song.setArtist(self.artist)
self.songs.append(song)

def deleteSong(self, name):

x = 0
while x < self.songs.__len__():
cname = self.songs[x].getName()

if cname == name:
del self.songs[x]
return True

x += 1

return False

def __str__(self):
val = self.name + '\t'

for x in self.songs:
val += x.__str__() + " | "

return val
def getSongs(self):
return self.songs







class Artist:

name = ''
songs = False #this is only really for deleting songs
albums = False
unknownAlbum = '' #set in init

def __init__(self, name):
if not self.songs: self.songs = []
if not self.albums: self.albums = []
self.unknownAlbum = Album('unknown')
self.albums.append(self.unknownAlbum)
self.name = name
self.unknownAlbum.setArtist(self.name)

def setName(self, name):
for x in albums:
self.name = name

def getName(self):
return self.name

def addSong(self, song): #must be song class
##~ print 'NAME:\t:'+self.name
song.setArtist(self.name)

a = song.getAlbum()
for x in self.albums:
if a == x.getName():
##~ print '--------'
##~ print a + ' ' + x.getName()
##~ print x
##~ print song
##~ print '--------'
x.addSong(song)
print 'ya'
self.songs.append(song)
print self.songs
##~ print '********'
##~ print x
##~ print self.unknownAlbum
##~ print '********'
return

if a == '':

self.unknownAlbum.addSong(song)
#print "yo " + song.__str__()
a = True
self.songs.append(song)
return

elif a != True:
#print 'yea ' + song.__str__()
newAlbum = Album(a)
newAlbum.setArtist(self.name)
self.albums.append(newAlbum)

print self.addSong(song)
##~ print "@"+
self.albums[self.albums.__len__()-1].__str__()
return



def deleteSong(self, name): #give song name

x = 0
while x < self.songs.__len__():
cname = self.songs[x].getName()

if cname == name:
a = self.songs[x].getAlbum()

for calbum in self.albums:
if a == calbum.getName():
calbum.deleteSong(name)

del self.songs[x]
return True

x += 1

return False

def addAlbum(self, Album): #must be album class
aSongs = Album.getSongs()
for song in aSongs: self.songs.append(song)
Album.setArtist(self.name)
self.albums.append(Album)

def deleteAlbum(self, name):

x = 0
while x < self.albums.__len__():
cname = self.albums[x].getName()

if cname == name:

cSongs = self.albums[x].getSongs()

for cSong in cSongs:
self.deleteSong(cSong.getName())

del self.albums[x]
return True

x += 1

return False

def __str__(self):
val = ''
for x in self.albums:
val += x.__str__() + '\n'


return val

def getSongs(self): return self.songs
def getAlbums(self): return self.albums

#test stuff
s = Song('song 1')
s2 = Song('song 2')

a = Album('hey')
a.addSong(s)

art = Artist('Tim')

art.addAlbum(a)
art.addSong(s2)

print art
art.deleteSong(s.getName())

print art
---------------------------------------------------------------------


cheers
Tim Henderson

 

[Tim Henderson]

actually i bunch of the code that was posted above has been changed including this.

code:
--------------------------------------------------------------
class Artist:


def __init__(self, name):
self.songs = []
self.albums = []
self.unknownAlbum = Album('unknown')
self.albums.append(self.unknownAlbum)
self.name = name
self.unknownAlbum.setArtist(self.name)