Personal book library

Question

Very new to OOP with Python - and feel like I still don't totally get OOP. However I thought I'd put together a personal online book library to practice inheritance, methods, docstrings etc.

Program allows one to add books (comics or novels - with comics inheriting from novels) to one's own library, and create a custom reading lists.

There are a few different methods. For example, a library method which will return all books over a specified length.

Note that I've removed quite a lot from this, just so it's not 5 pages long! I would normally have several different types of books, attributes such as Author etc. But hopefully from the below you get a good idea of the structure.

Speaking of structure, I would just like to know if mine is any good. Also there is an issue whereby if you remove a book from the library, it will still show up in any reading lists I created. I don't know if there's a way to deal with this, or if I'm thinking about this the wrong way.

class Library:
    
    def __init__(self, books = None):
        if books is None:
            self.books = []
        else:
            self.books = books

    def add_book(self, book):
        self.books.append(book)
        
    def remove_book(self, book):
        self.books.remove(book)
        
    def pages_over_n(self, number):
        """Returns books that have more than specified number of pages"""
        return [book for book in self.books if book.pages > number]
    
    def get_library(self):
        """Returns all books in Library"""
        return self.books
    
    def __repr__(self):
        return f"Library({self.books})"
    
    def __str__(self):
        return f"Library has {len(self.books)}"

class Novel:
    
    def __init__(self, title, pages, publicationDate):
        self.title = title
        self.pages = pages
        self.publicationDate = publicationDate
        
    def __repr__(self):
        return f"Novel({self.title}, {self.pages}, {self.publicationDate})"
    
    def __str__(self):
        return f"Media: {self.title}"
    
class ComicBook(Novel):
    
    def __init__(self, title, pages, publicationDate, artist):
        super().__init__(title, pages, publicationDate)
        self.artist = artist
        
    def get_artist(self):
        return f"Artist: {self.artist}"
        
    def __repr__(self):
        return (f"ComicBook({self.title}, {self.pages}, " 
                f"{self.publicationDate}, {self.artist})")
    
class ReadingList:
    
    def __init__(self):
        self.list = []
    
    def add_book(self, book):
        """Add book to reading list"""
        self.list.append(book)
        
    def remove_book(self, book):
        """Remove book from reading list"""
        self.list.remove(book)

    def total_pages(self):
        """Returns total pages in reading list"""
        total = 0
        for book in self.list:
            total += book.pages
        return total

    def __repr__(self):
        return f"ReadingList({self.list})"
    
    def __str__(self):
        return f"Reading list of size {len(self.list)}"

# Initialise Library
library = Library()

# Create a few books
novel1 = Novel("Harry Potter", 500, 1991)
novel2 = Novel("LotR", 1000, 1960)
novel3 = Novel("Game of Thrones", 2000, 2018)
comic1 = ComicBook("Batman", 100, 2020, "Stan Lee")

# Add books to library.
library.add_book(novel1)
library.add_book(novel2)
library.add_book(novel3)
library.add_book(comic1)

# Create a new reading list.
readingListOne = ReadingList()

# Add a couple of books to reading list.
readingListOne.add_book(novel1)
readingListOne.add_book(comic1)

Answer 1

General Observations

I'm being 100% honest here, When I just give a glance at your code there is nothing that is immediately horrifying with it.

• You shouldn't use variable/function names that are already defined in Python. Your class ReadingList has an attribute list. This is a bad idea as there is already a list keyword that can clash with your definition

Code Structure

You have tried to implement the idea of inheritance, although it is fine, it will be much better to have your base class as Book.

This makes more sense due to the fact that novels, and comic books are types of books. And hence can share attributes and methods.

Your Book class can have attributes and methods like

• Author
• Name
• Stuff which is common in all types of books

And your child class Novel can have

• Sutff which is special in Novel

A very very basic implementation

class Book():
    def __init__(self,author,name):
        self.author = author
        self.name = name

    def print_book(self):
        print(f"{self.name} by {self.author}")




class Novel(Book):
    def __init__(self,author,name):
        Book.__init__(self,author,name)

    def display(self):
        print(f"The Novel {self.name}")
        # Novel stuff which isn't common in all books

class Comic(Book):
    def __init__(self,author,name):
        Book.__init__(self,author,name)

    def display(self):
        print(f"The comic book {self.name}")
        # Comic book stuff which isn't common in all books
        

nov = Novel("J.K Rowling","Harry Potter")
nov.print_book()

The point is that your Novel class is basically a Book which as an author, pages, etc. But it also has something which other books like Comic don't. This way you don't need to create an entirely new class, but you can simply inherit the common from the Book class and add the extra stuff in Novel

---

Creating a library

You can stop here if you want to, and consider your Book class as a library. But if you want to get crazier, you can have another class called Magazine and that would be the child class of parent Library. Here is what that would look like.

But this would get quite complicated, moreover, it wouldn't make a lot of sense to inherit Book from Library. Hence it is a good idea to keep Library as a separate that will merely control the books.

class Library():
    def __init__(self,book_list = []):
        self.book_list = book_list

    def new_book(self,book):
        self.book_list.append(book)

    def remove_book(self,name): # book is the name of the book
        for book in self.book_list:
            if book.name == name:
                self.book_list.remove(book)
                break


    def print_books(self):
        for book in self.book_list:
            print(f"Book {book.name} by {book.author}")


class Book():
    def __init__(self,author,name):
        self.author = author
        self.name = name

    def print_book(self):
        print(f"{self.name} by {self.author}")




class Novel(Book):
    def __init__(self,author,name):
        Book.__init__(self,author,name)

    def display(self):
        print(f"The Novel {self.name}")
        # Novel stuff which isn't common in all books

class Comic(Book):
    def __init__(self,author,name):
        Book.__init__(self,author,name)

    def display(self):
        print(f"The comic book {self.name}")
        # Comic book stuff which isn't common in all books


my_lib = Library()
my_lib.new_book(Novel("J.K Rowling","Harry Potter"))
my_lib.new_book(Novel("Aryan Parekh","Stack"))

my_lib.remove_book("Stack")

my_lib.print_books()

Answer 2

Simplify the constructor of class Library

You have a default value of None for the parameter books, but then immediately proceed to convert that None to an empty list. Why not use [] as the default value?

class Library:
    def __init__(self, books = []):
        self.books = list(books)

    ...

Note that to prevent the issue of mutable default arguments as mentioned by ojdo, you should make a copy of the initial list.

Be consistent

You take an optional list of books in the constructor of class Library, but there is no such thing in class ReadingList. That is a bit surprising.

Also be consistent with naming things. In class Library, the list of books is called books, but in class ReadlingList you named it list. I would name it books in both cases.

Also be consistent with the names of functions:

• get_library() should be get_books(), since it only returns the list of books, not an object of type Library.
• pages_over_n() should be get_books_with_over_n_pages() or something similar, to indicate that it returns Books, not pages.
• total_pages() should be renamed get_total_pages() or calculate_total_pages(). Prefer using verbs for function names, and nouns for variables.

And I also wonder why a ComicBook has an artist, but a Novel has no author? And why is there a get_artist() but no get_title(), get_publication_date() and so on? Making everything consistent will make using your classes much easier.

Removing books from the Library

As you noticed, you can remove a book from the library, but it won't be removed from any reading lists. But that is fine. Why would a book magically disappear from a real life reading list if your library removes that book? Note that your code is perfectly fine, Python won't actually delete the Book object from memory until the last reference to it is gone.

However, if you want a book to disappear from the ReadingList when it is removed from the Library, then somehow there needs to be some communications between those two objects. There are two options:

1. A Library keeps a list of ReadingLists that reference its Books, and when you call remove_book() on a Library, it will in turn iterate over all the ReadingLists and call remove_book() on them as well.

2. A ReadingList keeps a reference to the Library whose Books it contains. Whenever you call a member function that accesses the list of books on the reading list, this member function first has to filter out all the books that are no longer in the Library.

Answer 3

Since Library and ReadingList are just different types of lists of books, I would opt for define a generic BookList as a base class from which Library and ReadingList both inherit.

Doing this saves you from the repetition of having to define common methods like the following, which both of your classes already do. Here are some methods from the Library class.

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

def add_book(self, book):
    self.books.append(book)
    
def remove_book(self, book):
    self.books.remove(book)

And here are the analogous methods from the ReadingList class.

def __init__(self):
    self.list = []

def add_book(self, book):
    """Add book to reading list"""
    self.list.append(book)
    
def remove_book(self, book):
    """Remove book from reading list"""
    self.list.remove(book)

As G. Sliepen said, you should simplify the Library constructor to match the ReadingList constructor. After you do this, the methods are exactly the same.

By defining a generic book list class, not only do you not have to keep redefining common functionality, you can also take advantage of polymorphic functions.

For example, rather than making each class responsible for outputting a string representation of itself, you could define a BookListHTMLWriter that takes a book list item (either a ReadingList or a Library, both are fine when they're derived from the same BookList object), and outputs an HTML table representation.

Likewise, you could also define a BookListXMLWriter that outputs the XML representation of a BookList object or a subclass thereof. You could take this OOP-refactoring even further by then refactoring the functionality common to the BookListHTMLWriter and BookListXMLWriter classes and derive them from a base BookListWriter base class.

Optimistically, object-oriented programming can be a bit of an acquired taste (and an esoteric one, at that), but once you start thinking in these terms it's a useful mental model, even if it's not your only one. Or you preferred one, even.

---

Regarding your point about how removing a book from your library does not remove it from your reading list(s), I agree with previous responses that this makes sense. I think the problem that you're alluding to might be the question of referential integrity, which I noticed, too.

You've obviously already noticed that Library and ReadingList are both lists of Books. When reading over code, I always start picturing the corresponding database schema, and the problem I think you're referring to is that there should be a main list of already created books from which you can add to a personal library and/or reading list, presumably depending on whether you own the book. I would call this main list a "catalog" or something to differentiate this main list from a list of a particular type belonging to a particular user.

Moreover, this catalog would simply be all of the records in a books table in your database.

To implement a database for this program, you could start by creating the books and users tables, and then creating a third table consisting only of a user's id and a book's id. Querying this link table for a given user id would give you all of the books this person owns.

If you wanted users to be able to create arbitrary libraries based not just on whether they own a book, you would only need to create a reading_lists table with a user id as a field to indicate the owner, and then create another link table using a list id and a book id.

This would solve your referential integrity problem by preventing the deletion of a book record unless you specify an appropriate ON DELETE action in the books table schema.

Subclassing Books into Novels and Magazines complicates things because if the inheritance hierarchy imbues complex enough differences on your subclasses, your objects no longer map nicely into single tables (assuming you want a reasonably-normalizable schema).

If you hit this point, a good entity-relational mapping tool can save you hours of frustration.