views:

84

answers:

3

Hi, I was writing this little piece of code as an exercise in object-oriented programming.

Here I'm trying to define a house as a list of rooms and each room as a list of devices (lamps, for example).

First I created all the objects and them appended the two rooms to the house and a different device to each room. Pretty basic.

The problem is that it seems that the device is being appended to both rooms. Why is that?

The code:

#! /usr/bin/python

class House:
     def __init__(self, rooms = list()):
       self.rooms = rooms
       print('house created')


class Room:
     def __init__(self, name = 'a room', devs = list()):
      self.name = name
      self.devs = devs
      print('room ' + self.name + ' created')


class Device:
     def __init__(self, name = 'a device'):
       self.name = name
       print('device ' + self.name + ' created')


def main():
     #1
     h = House()
     r1 = Room(name = 'R1')
     r2 = Room(name = 'R2')
     d1 = Device(name = 'lamp1')
     d2 = Device(name = 'lamp2')

     #2
     h.rooms.append(r1)
     h.rooms.append(r2)

     for room in  h.rooms:
       print room.name

     print h.rooms[0]
     print h.rooms[1]
     h.rooms[1].devs.append(d1)

     #3
     for room in h.rooms:
       print room.name
       for dev in room.devs:
         print('room ' + room.name + ' > ' + dev.name)
         print room
         print dev


if __name__ == '__main__' : main()

And the output.

house created
room R1 created
room R2 created
device lamp1 created
device lamp2 created
R1
R2
<__main__.Room instance at 0xb7d8a58c>
<__main__.Room instance at 0xb7d8a5ac>
R1
room R1 > lamp1
<__main__.Room instance at 0xb7d8a58c>
<__main__.Device instance at 0xb7d8a5cc>
R2
room R2 > lamp1
<__main__.Room instance at 0xb7d8a5ac>
<__main__.Device instance at 0xb7d8a5cc>

Note that the same instance of d1 is in both rooms, r1 and r2.

+8  A: 

Default parameter values for functions are evaluated only once. This means that all instances of House will use the same list instance for self.rooms (if rooms parameter wasn't given in construction). In the same manner, all instances of Room will share the same list for self.devs.

To solve this, write the code like this:

def __init__(self, rooms = None):
    if rooms is None:
        rooms = []
    self.rooms = rooms
    print('house created')

And the same thing for the other classes.

interjay
+1: Golden Rule: Never use mutable objects as default values.
S.Lott
A: 
def __init__(self, name = 'a room', devs = list()):
    self.name = name
    self.devs = devs
    print('room ' + self.name + ' created')

When you do this list() actually is always the same list. You don't get a new empty list each time the constructor is called, you get the same empty list. To fix that you'll want to make a copy.

Also list() is more idiomatically written as [].

def __init__(self, name='a room', devs=[]):
    self.name = name
    self.devs = list(devs)
    print('room ' + self.name + ' created')
John Kugelman
+3  A: 

The default argument is evaluated once, at the point of declaration of the method. That value is then used in all calls to the method.

There are other question on stackoverflow exploring the reasons for this design and how to best avoid these mutable default arguments.

sth