Veneer project - buy_artwork function won't work

bbalagia · February 12, 2019, 4:36am

https://www.codecademy.com/paths/computer-science/tracks/cspath-cumulative-art-marketplace/modules/cspath-veneer/projects/veneer

Hello,
I’m working on the Veneer project and am having trouble getting my buy_artwork function to work. It seems to have no effect whatsoever. Everything up to this point was working so if anyone knows what could be causing this, let me know. Thanks!

class Art:
  def __init__(self, artist, title, medium, year, owner):
    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 Marketplace:
  def __init__(self):
    self.listings = []
  
  def add_listing(self, new_listing):
    self.listings.append(new_listing)
  
  def remove_listing(self, old_listing):
    self.listings.remove(old_listing)
    
  def show_listings(self):
    for listing in self.listings:
      print(listing)
      
class Client:
  def __init__(self, name, location, is_museum):
    self.name = name
    self.location = location
    self.is_museum = is_museum
    
  def sell_artwork(self, artwork, price):
    if artwork.owner == self:
      new_listing = Listing(self, artwork, price)
      veneer.add_listing(new_listing)

  def buy_artwork(self, artwork):
    if artwork.owner != self:
      art_listing = None
      for listing in veneer.listings:
        if listing.art == artwork:
          art_listing = listing
          break
    if art_listing != None:
      art_listing.art.owner = self
      veneer.remove_listing(art_listing)

class Listing:
  def __init__(self, art, price, seller):
    self.art = art
    self.price = price
    self.seller = seller
   
  def __repr__(self):
    return "%s, %s" %(self.art.name, self.price)
    
edytta = Client("Edytta Halpirt", "Private Collection", False)
moma = Client("The MOMA", "New York", True)   
    
girl_with_mandolin = Art("Picasso, Pablo", "Girl with a Mandolin (Fanny Tellier)", "oil on canvas", "1910", (edytta))

veneer = Marketplace()

edytta.sell_artwork(girl_with_mandolin, "$6M (USD)")

moma.buy_artwork(girl_with_mandolin)

print(girl_with_mandolin)

appylpye · February 12, 2019, 8:54pm

Hi @bbalagia

Consider this statement within the sell_artwork method of Client:

      new_listing = Listing(self, artwork, price)

What objects are being passed, via the arguments, to the __init__ method of Listing? For example, what does self represent in that statement?

Edited on February 12, 2019 to revise the questions asked above

bbalagia · February 18, 2019, 4:33pm

self represents the Listing class, right?

appylpye · February 18, 2019, 6:20pm

The statement that we are discussing is part of the sell_artwork method of the Client class. That method has this header:

  def sell_artwork(self, artwork, price):

Therefore, in the statement under consideration, self represents the instance of Client from which the sell_artwork method is being called, rather than the instance of Listing that is involved in the sale. The instance of Client represented by self is the seller of the work of art.

Since we are creating an instance of Listing, we need to consider what arguments to pass to the __init__ method of Listing. This is the header of that method:

  def __init__(self, art, price, seller):

Note that the final parameter is seller. That parameter needs to receive the reference to the Client who is selling the art, namely self. So, here is how we make the call, matching the arguments to the parameters of the appropriate __init__ method:

      veneer.add_listing(Listing(artwork, price, self))

sssasha.moroz · June 1, 2019, 6:52pm

I have the similar problem, but your answer would better fit the problem with sell_artwork. In this case we having problems with but_artwork.

Here is my code:

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

class Marketplace:
  def __init__(self):
    self.listings = []
  
  def add_listing(self, new_listing):
    self.listings.append(new_listing)
    
  def remove_listing(self, del_listing):
    self.listings.remove(del_listing)
    
  def show_listings(self):
    for listing in self.listings:
    	print(listing)
    
veneer = Marketplace()

class Client:
    def __init__(self, name, location, is_museum):
        self.name = name
        self.location = location	
        self.is_museum = is_museum
  
    def sell_artwork(self, artwork, price):
        if artwork.owner == self:
            new_listing = Listing(artwork, price, self)
            veneer.add_listing(new_listing)
      
    def buy_artwork(self, artwork):
        if artwork.owner != self:
            art_listing = None
            for listing in veneer.listings:
                if listing.art != artwork:
                    art_listing = listing
                    break
            if art_listing != None:    
              art_listing.art.owner = self
              veneer.remove_listing(art_listing)
              
class Listing:
  def __init__(self, art, price, seller):
    self.art = art
    self.price = price
    self.seller = seller
  
  def __repr__(self):
    return "{art} has a value of {price}".format(art=self.art, price=self.price)

#buyer
edytta = Client("Edytta Halpirt", "Private Collection", False)
#seller
moma = Client("MOMA", "New York", True)
#art
girl_with_mandolin = Art("Picasso, Pablo", "Girl with a Mandolin (Fanny Tellier)", "oil on canvas", 1910, edytta)

#PROCESS
edytta.sell_artwork(girl_with_mandolin, "$6M USD")
veneer.show_listings()
moma.buy_artwork(girl_with_mandolin)
print(girl_with_mandolin)
veneer.show_listings()

What I get after calling

moma.buy_artwork(girl_with_mandolin)

is this:

Picasso, Pablo. “Girl with a Mandolin (Fanny Tellier)”. 1910, oil on canvas. Owner: Edytta Halpirt, Private Collection has a value of $6M USD
Picasso, Pablo. “Girl with a Mandolin (Fanny Tellier)”. 1910, oil on canvas. Owner: Edytta Halpirt, Private Collection
Picasso, Pablo. “Girl with a Mandolin (Fanny Tellier)”. 1910, oil on canvas. Owner: Edytta Halpirt, Private Collection has a value of $6M USD

(Basically not changing the owner and not removing the artwork from listing.)

What I think is happening is that for some reason this IF block isn’t triggering.

 if art_listing != None:    
              art_listing.art.owner = self
              veneer.remove_listing(art_listing)

mtf · June 1, 2019, 7:07pm

The instance is named veneer, but the method should not know that. We cannot use the instance name in the method. What if there are several instances? We should refer to the owner instance as self.

sssasha.moroz · June 1, 2019, 7:52pm

If I change veneer.remove_listing(art_listing) to self.remove_listing(art_listing) I would refer to the Client class, which doesn’t have remove_listing() Method. I would recieve the following Error:

Traceback (most recent call last):
File “script.py”, line 67, in
edytta.sell_artwork(girl_with_mandolin, “$6M USD”)
File “script.py”, line 37, in sell_artwork
self.add_listing(new_listing)
AttributeError: ‘Client’ object has no attribute ‘add_listing’

And in the video provided in “Get Help” they use veneer.remove_listing(art_listing).

appylpye · June 1, 2019, 8:23pm

@sssasha.moroz,

The buy_artwork method needs to search through veneer.listings in order to find the desired artwork. However, you have this in the for loop that is aimed at finding the match:

                if listing.art != artwork:

Is that what really needs to be done?

patrickd314 · June 1, 2019, 8:30pm

    def buy_artwork(self, artwork):
        if artwork.owner != self:
            art_listing = None
            for listing in veneer.listings:
                if listing.art != artwork:
                    art_listing = listing
                    break
            if art_listing != None:    
              art_listing.art.owner = self
              veneer.remove_listing(art_listing)

The variable art_listing remains None if the listing is the artwork in question. That is, the expression
if listing.art != artwork
returns False, art_listing stays None, and the “remove” clause won’t be entered.

appylpye · June 1, 2019, 8:54pm

The instructions have the user create a system wherein all transactions are performed within a single instance of Marketplace. We could, of course, create additional instances, however that would require a redesign of the system to enable the seller to choose where to list the work to be sold. Consequently, the buyer would need to query all instances of Marketplace in order to determine whether the desired work is available.

mtf · June 1, 2019, 9:06pm

Ah, I see. Thanks for clarifying.

appylpye · June 1, 2019, 9:18pm

The sole problem in @sssasha.moroz’s original code is the condition given in that if statement within the for loop. To identify a match, we need to do this instead:

                if listing.art == artwork:

With that change made, the output becomes:

Picasso, Pablo. "Girl with a Mandolin (Fanny Tellier)". 1910, oil on canvas. 
Owner: Edytta Halpirt, Private Collection has a value of $6M USD
Picasso, Pablo. "Girl with a Mandolin (Fanny Tellier)". 1910, oil on canvas. 
Owner: MOMA, New York

Now, the ownership has been transferred correctly and veneer.listings is empty.

gnartistic · August 15, 2019, 9:59am

class Art:
  def __init__(self, artist, title, medium, year, owner):
    self.artist = artist
    self.title = title
    self.medium = medium
    self.year = year
    self.owner = owner
    
  def __repr__(self):
    return str("The artist: {}, Title of the piece: {}, Medium: {}, Year of creation: {}, Owner: {}".format(self.artist, self.title, self.medium, str(self.year), self.owner))
  
class Marketplace:
  def __init__(self):
    self.listings = []

  def add_listing(self, new_listing):
    self.listings.append(new_listing)
    
  def remove_listing(self, old_listing):
    self.listings.remove(old_listing)
    
  def show_listing(self):
    return(print(self.listings))
  
class Client:
  def __init__(self, name, location, is_museum):
    self.name = name
    self.is_museum = is_museum
    if self.is_museum == True:
      self.location = location
    else: self.location = "Private Collection"
  
  def __repr__(self):
    return "{}, {}".format(self.name, self.location)
  
  def sell_artwork(self, artwork, price):
    if artwork.owner == self:
      new_listing = Listing(artwork, price, self)
      veneer.add_listing(new_listing)
  
  def buy_artwork(self, artwork):
    if artwork.owner != self:
      art_listing = None
      for listing in veneer.listings:
        if listing.art == artwork:
          art_listing = listing
          break
        if art_listing != None:
          art_listing.art.owner = self
          veneer.remove_listing(art_listing)
          
          
        
      
   
class Listing:
  def __init__(self, art, price, seller):
    self.art = art
    self.price = price
    self.seller = seller
    
  def __repr__(self):
    return "{}, {}".format(self.art, self.price)
  
veneer = Marketplace()

edytta = Client("Edytta Halprit", None, False)

moma = Client("The MOMA", "New York", True)

girl_with_mandolin = Art("Picasso Pablo", "Girl with a Mandolin (Fanny Tellier)", "oil on canvas", 1910, edytta)

# print(girl_with_mandolin)

edytta.sell_artwork(girl_with_mandolin, "$6M (USD)")

#print(veneer.show_listing())

moma.buy_artwork(girl_with_mandolin)

print(girl_with_mandolin)
print(veneer.listings)
print(veneer.show_listing())

I’m probably just dumb and my question was answered but I still can’t seem to get the buy_artwork method to work </3 If anyone can help a poor noob I’d be forever in your debt

appylpye · August 15, 2019, 11:50pm

Hi @gnartistic,

The purpose of the for loop in the buy_artwork method is to determine whether artwork is actually listed for sale. If it is, then ownership can be transferred, otherwise it cannot.

The purpose of this if block is to perform the transfer of the artwork if, and only if, the for loop has determined that it is, in fact, for sale:

        if art_listing != None:
          art_listing.art.owner = self
          veneer.remove_listing(art_listing)

Therefore, that if block must follow the for loop rather than be contained within it. However, you have the if block indented to a degree that includes it within the for loop. Remove a level of indentation from the if block to correct the problem.

gnartistic · August 16, 2019, 5:47am

Thank you senpai