Python Scoping/Static Misunderstanding

Question

I'm really stuck on why the following code block 1 result in output 1 instead of output 2?

Code block 1:

class FruitContainer:
       def __init__(self,arr=[]):
           self.array = arr
       def addTo(self,something):
           self.array.append(something)
       def __str__(self):
           ret = "["
           for item in self.array:
               ret = "%s%s," % (ret,item)
           return "%s]" % ret

arrayOfFruit = ['apple', 'banana', 'pear']
arrayOfFruitContainers = []

while len(arrayOfFruit) > 0:
   tempFruit = arrayOfFruit.pop(0)
   tempB = FruitContainer()
   tempB.addTo(tempFruit)
   arrayOfFruitContainers.append(tempB)

for container in arrayOfFruitContainers:
   print container 

**Output 1 (actual):**
[apple,banana,pear,]
[apple,banana,pear,]
[apple,banana,pear,]

**Output 2 (desired):**
[apple,]
[banana,]
[pear,]

The goal of this code is to iterate through an array and wrap each in a parent object. This is a reduction of my actual code which adds all apples to a bag of apples and so forth. My guess is that, for some reason, it's either using the same object or acting as if the fruit container uses a static array. I have no idea how to fix this.

Answer 1

You should never use a mutable value (like []) for a default argument to a method. The value is computed once, and then used for every invocation. When you use an empty list as a default value, that same list is used every time the method is invoked without the argument, even as the value is modified by previous function calls.

Do this instead:

def __init__(self,arr=None):
    self.array = arr or []

Answer 2

As Ned says, the problem is you are using a list as a default argument. There is more detail here. The solution is to change __init__ function as below:

       def __init__(self,arr=None):
           if arr is not None:
               self.array = arr
           else:
               self.array = []

Answer 3

Your code has a default argument to initialize the class. The value of the default argument is evaluated once, at compile time, so every instance is initialized with the same list. Change it like so:

def __init__(self, arr=None):
    if arr is None:
        self.array = []
    else:
        self.array = arr

I discussed this more fully here: http://stackoverflow.com/questions/1495666/how-to-define-a-class-in-python/1495740#1495740

Answer 4

A better solution than passing in None — in this particular instance, rather than in general — is to treat the arr parameter to __init__ as an enumerable set of items to pre-initialize the FruitContainer with, rather than an array to use for internal storage:

class FruitContainer:
  def __init__(self, arr=()):
    self.array = list(arr)
  ...

This will allow you to pass in other enumerable types to initialize your container, which more advanced Python users will expect to be able to do:

myFruit = ('apple', 'pear') # Pass a tuple
myFruitContainer = FruitContainer(myFruit)
myOtherFruit = file('fruitFile', 'r') # Pass a file
myOtherFruitContainer = FruitContainer(myOtherFruit)

It will also defuse another potential aliasing bug:

myFruit = ['apple', 'pear']
myFruitContainer1 = FruitContainer(myFruit)
myFruitContainer2 = FruitContainer(myFruit)
myFruitContainer1.addTo('banana')
'banana' in str(myFruitContainer2)

With all other implementations on this page, this will return True, because you have accidentally aliased the internal storage of your containers.

Note: This approach is not always the right answer: "if not None" is better in other cases. Just ask yourself: am I passing in a set of objects, or a mutable container? If the class/function I'm passing my objects in to changes the storage I gave it, would that be (a) surprising or (b) desirable? In this case, I would argue that it is (a); thus, the list(...) call is the best solution. If (b), "if not None" would be the right approach.