[Tutor] Methods defined in my class are affecting all the objects at runtime.

Steven D'Aprano steve at pearwood.info
Sat Oct 29 17:33:44 CEST 2011


Brian Stovall wrote:
> Hello world!
> 
> I obviously don't understand something important and basic, but I am
> having trouble figuring it out myself... I am running python v3.2.2 on
> a Win XP machine.
[...]
> I had been testing it with single objects favorably, but when I
> instantiate two Pile objects, methods like .add or .shuffle affect all
> of the Pile objects in memory. At first I thought the objects were all
> initializing to the same space in memory, but it wasn't true.

The problem is not that all Pile objects are in the same space in 
memory, but that they all share the same cards.

If you call:

a = Pile()
b = Pile()
c = Pile()

then all three Piles a, b, c share the same list of cards. If instead 
you explicitly call Pile([]) each time then I bet the problem will go 
away. Try it and see.

The problem occurs in your __init__ method, where you use a default 
value of [] for cards:

     def __init__(self, cards = [], display_type = "STACK"):
         self.cards = cards
         self.display_type = display_type

The thing is, in Python the default value is only calculated *once*, 
when the function or method is defined, and then reused each time. You 
can test this for yourself very easily:


import time

def test(t=time.asctime()):
     print(t)


Call test(), wait a few seconds, and call it again, and precisely the 
same time will be printed. The default value for t is calculated once, 
then reused.

Normally this is not a problem, but if the default value is a list or 
dict, it means that the method or function remembers state from one call 
to the next:

 >>> def test(x, alist=[]):
...     alist.append(x)  # Modify the list in place.
...     return alist
...
 >>> test(2)  # The first time you call the function, the default is []
[2]
 >>> test(3)  # But now the default is [2]
[2, 3]
 >>> test(5)  # And now the default is [2, 3]
[2, 3, 5]



You can easily fix this problem by making sure each Pile instance gets 
its own brand new empty list of cards, instead of sharing the same one:

     def __init__(self, cards=None, display_type="STACK"):
         if cards is None:
             cards = []
         self.cards = cards
         self.display_type = display_type



A couple of other minor points:

Your add method can be simplified to this:

     def add(self, card_list):
         self.cards.extend(card_list)


Also, your __str__ method builds up the string by repeated concatenation:

     def __str__(self):
         return_string = ""
         for i in self.cards:
             return_string = return_string + str(i) + "\n"
         return_string = return_string + str(self.display_type)
         return return_string

This is generally poor practice. Without going into a lot of detail, 
this risks being slow in Python. Very, very, VERY slow. Depending on the 
details of your operating system, exact version of Python, the specific 
strings being used, you may not notice any slowdown, but the risk is 
still there. The recommended way to build up a string out of many 
smaller substrings is like this:

     def __str__(self):
         # First build a list of all of the substrings.
         substrings = []
         for card in self.cards:
             # Please use a more descriptive name than "i"
             substrings.append(str(card))
         substrings.append(str(self.display_type))
         # Now join all of the substrings in one fast operation.
         return '\n'.join(substrings)


Hope this helps,


-- 
Steven


More information about the Tutor mailing list