Veneer Project - Step 35 - Create a wishlist

Hello everybody, I’m stuck regarding the update of a wishlist for a client. I’ve added a method to the class Client to add an artwork to a client’s wishlist (add_to_wishlist) but when I run it, it adds the artwork to all clients’ wishlist !

Exercise

Here is my code:

class Art:
  def __init__(self, artist, title, medium, year, owner = None):
    self.artist = artist
    self.title = title
    self.medium = medium
    self.year = year
    self.owner = owner
  def __repr__(self):
    return '{artist}. "{title}". {year}, {medium}. {owner}, {location}.'.format(artist=self.artist, title=self.title, year=self.year, medium=self.medium, owner=self.owner.name, location=self.owner.location)

class Listing:
  def __init__(self, art, price, seller):
    self.art = art
    self.price = price
    self.seller = seller
  def __repr__(self):
    return '{}: ${:,}'.format(self.art.title, self.price)

class MarketPlace:
  def __init__(self, listings):
    self.listings = listings
  def add_listing(self, new_listing):
    self.listings.append(new_listing)
  def remove_listing(self, listing_to_remove):
    self.listings.remove(listing_to_remove)
  def show_listings(self):
    for listing in self.listings:
      print(listing)

veneer = MarketPlace([])

class Client:
  def __init__(self, name, location, is_museum, wallet = 0, wishlist = []):
    self.name = name
    self.location = location
    self.is_museum = is_museum
    self.wallet = wallet
    self.wishlist = wishlist
  def __repr__(self):
    return '{}, {}. Budget: ${:,}, Wishlist: {}'.format(self.name, self.location, self.wallet, self.wishlist)
  def sell_artwork(self, artwork, price):
    if artwork.owner == self:
      veneer.add_listing(Listing(artwork, price, artwork.owner))
      self.wallet += price
  def buy_artwork(self, artwork):
    if artwork.owner != self:
      for listing in veneer.listings:
        if artwork == listing.art:
          artwork.owner = self
          self.wallet -= listing.price
          veneer.remove_listing(listing)
  def add_to_wishlist(self, artwork):
    if artwork.owner != self:
      self.wishlist.append(artwork.title)

edytta = Client('Edytta Halpirt', 'Private Collection', False)
girl_with_mandolin = Art('Picasso, Pablo', 'Girl with a Mandolin (Fanny Tellier)', 'oil in canvas', 1910, edytta)
edytta.sell_artwork(girl_with_mandolin, 6000000)
moma = Client('The MOMA', 'New York', True)
moma.buy_artwork(girl_with_mandolin)
edytta.add_to_wishlist(girl_with_mandolin)
print(edytta)
print(moma)

When I run the code, here is the output:

Edytta Halpirt, Private Collection. Budget: $6,000,000, Wishlist: ['Girl with a Mandolin (Fanny Tellier)']
The MOMA, New York. Budget: $-6,000,000, Wishlist: ['Girl with a Mandolin (Fanny Tellier)']

While I’ve only called the method add_to_wishlist for edytta, the program has also added the artwork to moma's wishlist !

Does someone have an idea why ?

2 Likes

It looks like that empty list assigned to wishlist by default in this __init__ method header of Client is shared between Client instances:

  def __init__(self, name, location, is_museum, wallet = 0, wishlist = []):

As a consequence, when a wishlist is altered, it is altered for all.

Within that __init__ method, change this:

    self.wishlist = wishlist

to this:

    self.wishlist = wishlist.copy()
5 Likes

Alternatively, if you don’t plan to ever pass in an argument for wishlist when you create an instance of your Client class, you can just have wishlist defined inside of your __init__ method:

class Client:
  def __init__(self, name, location, is_museum, wallet = 0):
    self.name = name
    self.location = location
    self.is_museum = is_museum
    self.wallet = wallet
    self.wishlist = []
3 Likes

Yeah, you could do what @el_cocodrilo suggested.

I must admit that it does bug me a little that an object used in the parameter list of a method gets shared amongst instances when the parameters themselves are local variables. We could, perhaps, rationalize this behavior of the Python interpreter by noting that methods themselves exist on the class level, are shared across instances, and therefore become defined before any instances are created. As part of that defining process, any objects that need to be created to serve as default values specified in the header get created. So, there is only one copy of those objects, regardless of how many instances get created later. When the default assigned to a parameter is a mutable object, such as a list, there may be some unwelcome consequences, if we are not careful.

3 Likes

^ This. So important.
For more info: https://docs.python.org/3.8/tutorial/classes.html#class-and-instance-variables

4 Likes

Here’s a simpler example that serves as reminder to be cautious:

def func(a, b = []):
  b.append(a)
  return b

h = func(4)
print(h)
i = func(5)
print(i)
j = func(6, [1, 2, 3])
print(h)
print(i)
print(j)

Output:

[4]
[4, 5]
[4, 5]
[4, 5]
[1, 2, 3, 6]

When func gets defined, the default [] gets created. It’s mutable, and its state persists between function calls.

4 Likes

If you’re curious you can technically check the defaults associated to a function object with-
func.__defaults__
Valid uses of mutable objects as default function parameters are a little less common though. I’m sure I saved a link about it one time but I’ve never considered using it myself. I’ll add an edit if I come across it again. Best to stick to the advice provided by the posts above.

3 Likes

That will do the trick ! Thanks !

Well, I didn’t realize this behavior until now ! Thanks for the insight, I will be more careful now !

1 Like

Also note that an assignment statement can create an alias, whereby two or more variable names refer to the same mutable object. See the following example.

a = [1, 2, 3]
# The following creates an alias
b = a
a.append(4)
print(a)
print(b)

# Also note the following
c = [11, 12, 13]
# The following list contains a reference to c
d = [c, 17, 18, 19]
d[0].append(20)
print(c)
print(d[0])

Output:

[1, 2, 3, 4]
[1, 2, 3, 4]
[11, 12, 13, 20]
[11, 12, 13, 20]
2 Likes