Python: Preventing duplication of data when using dictionaries and lists

Question

Hello Stack Overflow!

I am executing a simple command in a program that compiles a report of all the books contained in a library. The library contains a list of shelves, each shelves contains a dictionary of books. However, despite my best efforts, I am always duplicating all my books and placing them on every shelf, instead of the shelf I've instructed the program to place the book on.

I expect I have missed out on some kind of fundamental rule with object creation and organization.

I believe the culprits are the enshelf and unshelf methods in the book class.

Thank you so much for your time, Jake

Code below:

class book():   

    shelf_number = None

    def __init__(self, title, author):
        super(book, self).__init__()
        self.title = title
        self.author = author

    def enshelf(self, shelf_number):
        self.shelf_number = shelf_number
        SPL.shelves[self.shelf_number].books[hash(self)] = self

    def unshelf(self):  
        del SPL.shelves[self.shelf_number].books[hash(self)]
        return self

    def get_title(self):
        return self.title

    def get_author(self):
        return self.author

class shelf():

    books = {}

    def __init__(self):
        super(shelf, self).__init__()

    def get_books(self):
        temp_list = []

        for k in self.books.keys():
            temp_list.append(self.books[k].get_title())
        return temp_list

class library():

    shelves = []

    def __init__(self, name):
        super(library, self).__init__()
        self.name = name

    def make_shelf(self):
        temp = shelf()
        self.shelves.append(temp)

    def remove_shelf(shelf_number):
        del shelves[shelf_number]

    def report_all_books(self):

        temp_list = []

        for x in range(0,len(self.shelves)):
            temp_list.append(self.shelves[x].get_books())

        print(temp_list)

#---------------------------------------------------------------------------------------
#----------------------SEATTLE PUBLIC LIBARARY -----------------------------------------
#---------------------------------------------------------------------------------------

SPL = library("Seattle Public Library")                     

for x in range(0,3):
    SPL.make_shelf()

b1 = book("matterhorn","karl marlantes")
b2 = book("my life","bill clinton")
b3 = book("decision points","george bush")

b1.enshelf(0)
b2.enshelf(1)
b3.enshelf(2)

print(SPL.report_all_books())

b1.unshelf()
b2.unshelf()
b3.unshelf()

OUTPUT:

[['decision points', 'my life', 'matterhorn'], ['decision points', 'my life', 'matterhorn'], ['decision points', 'my life', 'matterhorn']] None [Finished in 0.1s]

..instead of [["decision points"],["my life"],["matterhorn"]]

Answer 1

1. Use dict.pop() instead of del.
2. Add self.books = {} to shelf's __init__. Don't declare books outside of the __init__, because if you do so, all of the instances of that class are going to refer to the same thing. Instead, this makes each instance have its own dictionary, which is of course what you want since a book can't be in two shelves at once.
3. Do the same for library and its shelves and book and its shelf_number.
4. Pass a library instance as an argument to enshelf and unshelf. When you refer to SPL from within your objects' methods, Python finds that there is no local SPL defined, so it searches for one outside of the local scope; but if you were to try to assign something to SPL or do some other sort of mutative business, you would get an UnboundLocalError.
5. Bonuses:
   • class book(object), class shelf(object), and class library(object). (Won't fix your problem, but you should do that anyway.)
   • You don't need to hash the keys before using them, they will be hashed (if they are hashable, but if you're hashing them, then they are).
   • There is no need to call super() unless you are inheriting from something, in which case you can delegate a method call to a parent or sibling using it - but you aren't doing that.
   • get_books() can be implemented as nothing more than return [self.books[k].get_title() for k in self.books.iterkeys()]
   • Likewise for report_all_books(): return [shlf.get_books() for shlf in self.shelves]. Note that I am not iterating over the indices, but rather over the elements themselves. Try for c in "foobar": print(c) in the interactive shell if you want to see for yourself.