Object-oriented student library

Question

This library management program can do following functions:

• Add a book to the library
• Display all available books
• Lend books to students
• Return books to the library
• Display students who have yet to return a book

Question: How do you refactor this code so that it follows OOP, reads better, is manageable and is pythonic? How can I write name functions and classes better? How do you know which data structure you need to use so as to manage data effectively?

Library class that contains a list of books

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

    def add_book(self, new_book):
        self._books.append(new_book)

    def display_books(self):
        if self._books:  # if list of books is not empty
            print("The books we have made available in our library are:\n")
            for book in self._books:
                print(book)
        else:
            print("Sorry, we have no books available in the library at the moment")

    def lend_book(self, requested_book):
        if requested_book in self._books:
            print("You have now borrowed \"%s\" " % requested_book)
            self._books.remove(requested_book)

        else:
            print("Sorry, \"%s\" is not there in our library at the moment" % requested_book)

StudentDatabase class (container for all students)

class StudentDatabase:
    def __init__(self):
        self._books = {}

    def get_student(self, name):
        if name not in self._books:
            return "Not Found"

        return self._books[name]

    def borrow_book(self, name, book):
        if name not in self._books:
            self._books[name] = list(book)
        else:
            self._books[name].append(book)

    def return_book(self, name, book):
        if book not in self._books[name]:
            print("You don't seem to have borrowed \"%s\"" % book)
        else:
            self._books[name].remove(book)

    def display_students_with_books(self):
        for name, books in self._books.items():
            if books:
                print("%s: %s" % (name, books))

Follow-up : Object-oriented student library

Answer 1

In addition to the below, I recommend reading Maarten Fabré's answer.

---

Edge case

Consider the following scenario:

book_tracking = StudentDatabase()
book_tracking.return_book("Student A", "Moby Dick")
# KeyError: 'Student A'

However, we don't need to do a try-except block to catch the student who never handled books before trying to return a book; Python provides defaultdict in the collections module to help us with this very problem:

from collections import defaultdict

class StudentDatabase:
    def __init__(self):
        self._books = defaultdict(list)

    # ...

Now, if anything tries to access an unassigned key of _books, it will simply return an empty list. This can subsequently simplify borrow_book():

    def borrow_book(self, name, book):
        self._books[name].append(book)

Interclass operability

Library and StudentDatabase don't seem to interact at all. If they're related, the student database listing book borrows should inform the library when a book is borrowed or returned. The most versatile way to do this at this stage is to add a library parameter to relevant methods.

class StudentDatabase:
    # ...

    def borrow_book(self, name, book, library):
        if library.lend_book(book):
            self._books[name].append(book)

    def return_book(self, name, book, library):
        if book not in self._books[name]:
            print("You don't seem to have borrowed \"%s\"" % book)
        else:
            library.add_book(book)
            self._books[name].remove(book)

This will require returning False when the book is not found:

class Library:

    # ...

    def lend_book(self, requested_book):
        if requested_book in self._books:
            print("You have now borrowed \"%s\" " % requested_book)
            self._books.remove(requested_book)
            return True

        else:
            print("Sorry, \"%s\" is not there in our library at the moment" % requested_book)
            return False

The implementation you currently have is very simple, so there's not much else to discuss in terms of design: it becomes more relevant when you have more moving parts. I do find it somewhat odd that it's only tracking by student, and not by book (i.e. Library could know how many copies of each book it had (int), to whom it's checked (a list perhaps with a custom CheckedOut class to store the person and the due date), and any other information pertaining to that book. A BookStatus class could store everything related to each book; the library would then contain a list of BookStatus objects.

Checking if a sequence is empty.

(Note I previously advocated using len(self._books) > 0 until Maarten Fabré (along with three people upvoting his comment) pointed out this suggestion contradicted PEP-8) The relevant passage is excerpted below.

For sequences, (strings, lists, tuples), use the fact that empty sequences are false.

Format string literals

You seem to be using printf-style string formatting. In Python 3, format strings are more flexible and would probably be considered more Pythonic. Python 3.6 adds format string literals, which are very compact. Here are some example uses:

• Original: "You have now borrowed \"%s\" " % requested_book
  • With f-string: f"You have now borrowed \"{requested_book}\"."
• Original: "Sorry, \"%s\" is not there in our library at the moment" % requested_book
  • With f-string: f"Sorry, \"{requested_book}\" is not there in our library at the moment"
• Original: "You don't seem to have borrowed \"%s\"" % book
  • With f-string: f"You don't seem to have borrowed \"{book}\""
• Original: "%s: %s" % (name, books)
  • With f-string: f"{name}: {books}"

Result

from collections import defaultdict

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

    def add_book(self, new_book):
        self._books.append(new_book)

    def display_books(self):
        if len(self._books) > 0:
            print("The books we have made available in our library are:\n")
            for book in self._books:
                print(book)
        else:
            print("Sorry, we have no books available in the library at the moment")

    def lend_book(self, requested_book):
        if requested_book in self._books:
            print(f"You have now borrowed \"{requested_book}\".")
            self._books.remove(requested_book)
            return True

        else:
            print(f"Sorry, \"{requested_book}\" is not there in our library at the moment")
            return False

class StudentDatabase:
    def __init__(self):
        self._books = defaultdict(list)

    def get_student(self, name):
        if name not in self._books:
            return "Not Found"

        return self._books[name]

    def borrow_book(self, name, book, library):
        if library.lend_book(book):
            self._books[name].append(book)

    def return_book(self, name, book, library):
        if book not in self._books[name]:
            print(f"You don't seem to have borrowed \"{book}\"")
        else:
            library.add_book(book)
            self._books[name].remove(book)

    def display_students_with_books(self):
        for name, books in self._books.items():
            if books:
                print(f"{name}: {books}")

Answer 2

Separate the logic and data from the presentation

return or yield, don't print. This way your code can be used in other programs that might reuse this.

set

Your Library and StudentDatabase uses lists to keep the books. Since the order is not important, but checking for containment and adding and removing is more important, a set is a more suited container than a list. This might give a problem when a student borrows more than 1 version of a book, but this can be solved by using real objects for the books instead of just strings for the titles.

Communicate failure with exceptions

def get_student(self, name):
    if name not in self._books:
        return "Not Found"
    return self._books[name]

No you have to always check for that special value "Not Found"

Better would be to raise an exception, or even just let the KeyError bubble up to the calling code

String literals

There are 4 ways to delimit string literals. ", ', """, '''. If you need both " and ' in the string itself, you can use one of the other ones to delimit the string. For example f'''{name} doesn't seem to have borrowed "{book}"'''

Together with Graham's comments, the result could be something like:

class StudentDataBaseException(Exception): pass
class NoStudent(StudentDataBaseException): pass
class NoBook(StudentDataBaseException): pass


class StudentDatabase:
    def __init__(self):
        self._books = defaultdict(set)

    def get_student(self, name):
        try:
            return self._books[name]
        except KeyError as exc:
            raise NoStudent(f"Student {name} not detected in Database") from exc

    def borrow_book(self, name, book, library):
        if library.lend_book(book):
            self._books[name].add(book)

    def return_book(self, name, book, library):
        if book not in self._books[name]:
            raise NoBook(f'''{name} doesn't seem to have borrowed "{book}"''')
        else:
            library.add_book(book)
            self._books[name].remove(book)

    def display_students_with_books(self):
        for name, books in self._books.items():
            if books:
                yield f"{name}: {books}" 

    def display_students_with_books(self):
        for name, books in self._books.items():
            if books:
                yield name, books

I added 2 versions of the display_students_with_books method, one yielding a str, the other yielding the actual name and books objects, so the user interface can take care of the formatting

Atomic operations

If an operation takes 2 steps, for example returning a book removes the book from a student's collection and adds it to the library, and something goes wrong with the second operation, the first operation needs to be cancelled. There are a number of ways to ensure this (try-except blocks,...) but this might be a concern.